diff --git a/java/google/registry/reporting/spec11/Spec11EmailUtils.java b/java/google/registry/reporting/spec11/Spec11EmailUtils.java index 908543ab6..5d2561019 100644 --- a/java/google/registry/reporting/spec11/Spec11EmailUtils.java +++ b/java/google/registry/reporting/spec11/Spec11EmailUtils.java @@ -14,16 +14,12 @@ package google.registry.reporting.spec11; -import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Throwables.getRootCause; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.io.Resources.getResource; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; import com.google.template.soy.SoyFileSet; @@ -87,40 +83,40 @@ public class Spec11EmailUtils { SoyTemplateInfo soyTemplateInfo, String subject, Set registrarThreatMatchesSet) { - // Null/empty email address shouldn't throw an exception, but we should warn if they occur. - ImmutableSet matchesWithoutEmails = - registrarThreatMatchesSet.stream() - .filter(matches -> isNullOrEmpty(matches.registrarEmailAddress())) - .collect(toImmutableSet()); - try { - for (RegistrarThreatMatches registrarThreatMatches : - Sets.difference(registrarThreatMatchesSet, matchesWithoutEmails)) { + ImmutableMap.Builder failedMatchesBuilder = + ImmutableMap.builder(); + for (RegistrarThreatMatches registrarThreatMatches : registrarThreatMatchesSet) { + try { + // Handle exceptions individually per registrar so that one failed email doesn't prevent the + // rest from being sent. emailRegistrar(date, soyTemplateInfo, subject, registrarThreatMatches); + } catch (Throwable e) { + failedMatchesBuilder.put(registrarThreatMatches, getRootCause(e)); } - } catch (Throwable e) { - // Send an alert with the root cause, unwrapping the retrier's RuntimeException + } + ImmutableMap failedMatches = failedMatchesBuilder.build(); + if (!failedMatches.isEmpty()) { + ImmutableList> failedMatchesList = + failedMatches.entrySet().asList(); + // Send an alert email and throw a RuntimeException with the first failure as the cause, + // but log the rest so that we have that information. + Throwable firstThrowable = failedMatchesList.get(0).getValue(); sendAlertEmail( String.format("Spec11 Emailing Failure %s", date), - String.format("Emailing spec11 reports failed due to %s", getRootCause(e).getMessage())); - throw new RuntimeException("Emailing spec11 report failed", e); - } - if (matchesWithoutEmails.isEmpty()) { - sendAlertEmail( - String.format("Spec11 Pipeline Success %s", date), - "Spec11 reporting completed successfully."); - } else { - logger.atSevere().log( - "Some Spec11 threat matches had no associated email addresses: %s", matchesWithoutEmails); - // TODO(b/129401965): Use only client IDs in this message - sendAlertEmail( - String.format("Spec11 Pipeline Warning %s", date), - String.format( - "No errors occurred but the following matches had no associated email: \n" - + "%s\n\n" - + "This should not occur; please make sure we have email addresses for these" - + " registrar(s).", - matchesWithoutEmails)); + String.format("Emailing Spec11 reports failed due to %s", firstThrowable.getMessage())); + for (int i = 1; i < failedMatches.size(); i++) { + // TODO(b/129401965): Use only client IDs in this message + logger.atSevere().withCause(failedMatchesList.get(i).getValue()).log( + "Additional exception thrown when sending email to registrar %s, in addition to the" + + " re-thrown exception", + failedMatchesList.get(i).getKey().registrarEmailAddress()); + } + throw new RuntimeException( + "Emailing Spec11 reports failed, first exception:", firstThrowable); } + sendAlertEmail( + String.format("Spec11 Pipeline Success %s", date), + "Spec11 reporting completed successfully."); } private void emailRegistrar( diff --git a/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java b/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java index d9a8e319c..11085e1b2 100644 --- a/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java +++ b/javatests/google/registry/reporting/spec11/Spec11EmailUtilsTest.java @@ -14,8 +14,9 @@ package google.registry.reporting.spec11; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; +import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchA; +import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchB; import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.sampleThreatMatches; import static google.registry.testing.JUnitBackports.assertThrows; import static org.mockito.Mockito.doThrow; @@ -25,11 +26,11 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.net.MediaType; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import javax.mail.MessagingException; @@ -46,6 +47,64 @@ import org.mockito.ArgumentCaptor; public class Spec11EmailUtilsTest { private static final ImmutableList FAKE_RESOURCES = ImmutableList.of("foo"); + private static final String DAILY_EMAIL_FORMAT = + "Dear registrar partner," + + "" + + "

Super Cool Registry conducts a daily analysis of all domains registered in its " + + "TLDs to identify potential security concerns. On 2018-07-15, the following domains " + + "that your registrar manages were flagged for potential security concerns:

" + + "" + + "" + + "" + + "%s" + + "
Domain NameThreat Type
" + + "

Please communicate these findings to the registrant and work with the " + + "registrant to mitigate any security issues and have the domains delisted.

" + + "" + + "Some helpful resources for getting off a blocked list include:" + + "
  • foo

" + + "" + + "You will continue to receive daily notices when new domains managed by your registrar " + + "are flagged for abuse, as well as a monthly summary of all of your domains under " + + "management that remain flagged for abuse. Once the registrant has resolved the " + + "security issues and followed the steps to have his or her domain reviewed and " + + "delisted it will automatically be removed from our reporting.

" + + "" + + "

If you would like to change the email to which these notices are sent please " + + "update your abuse contact using your registrar portal account.

" + + "" + + "

If you have any questions regarding this notice, please contact " + + "my-reply-to@test.com.

"; + private static final String MONTHLY_EMAIL_FORMAT = + "Dear registrar partner," + + "" + + "

Super Cool Registry previously notified you when the following " + + "domains managed by your registrar were flagged for potential security concerns.

" + + "

The following domains that you manage continue to be flagged by our analysis " + + "for potential security concerns. This may be because the registrants have not " + + "completed the requisite steps to mitigate the potential security abuse and/or have " + + "it reviewed and delisted.

" + + "" + + "" + + "" + + "%s" + + "
Domain NameThreat Type
" + + "" + + "

Please work with the registrant to mitigate any security issues " + + "and have the domains delisted.

" + + "" + + "Some helpful resources for getting off a blocked list include:" + + "
  • foo

" + + "" + + "You will continue to receive a monthly summary of all domains managed by your " + + "registrar that remain on our lists of potential security threats. You will " + + "additionally receive a daily notice when any new domains that are added to these " + + "lists. Once the registrant has resolved the security issues and followed the steps to " + + "have his or her domain reviewed and delisted it will automatically be removed from " + + "our monthly reporting.

" + + "" + + "

If you have any questions regarding this notice, please contact " + + "my-reply-to@test.com.

"; private SendEmailService emailService; private Spec11EmailUtils emailUtils; @@ -79,30 +138,13 @@ public class Spec11EmailUtilsTest { // We inspect individual parameters because Message doesn't implement equals(). verify(emailService, times(3)).sendEmail(contentCaptor.capture()); List capturedContents = contentCaptor.getAllValues(); - String emailFormat = - "Dear registrar partner,

Super Cool Registry previously notified you when the following " - + "domains managed by your registrar were flagged for potential security concerns." - + "

The following domains that you manage continue to be flagged by our analysis " - + "for potential security concerns. This may be because the registrants have not " - + "completed the requisite steps to mitigate the potential security abuse and/or have " - + "it reviewed and delisted.

" - + "%s
Domain NameThreat Type

Please work with the registrant to mitigate any security issues " - + "and have the domains delisted.

Some helpful resources for getting off a blocked " - + "list include:
  • foo

You will continue to receive a monthly summary " - + "of all domains managed by your registrar that remain on our lists of potential " - + "security threats. You will additionally receive a daily notice when any new domains " - + "that are added to these lists. Once the registrant has resolved the security issues " - + "and followed the steps to have his or her domain reviewed and delisted it will " - + "automatically be removed from our monthly reporting.

If you have any q" - + "uestions regarding this notice, please contact my-reply-to@test.com.

"; - validateMessage( capturedContents.get(0), "my-sender@test.com", "a@fake.com", Optional.of("my-reply-to@test.com"), "Super Cool Registry Monthly Threat Detector [2018-07-15]", - String.format(emailFormat, "a.comMALWARE"), + String.format(MONTHLY_EMAIL_FORMAT, "a.comMALWARE"), Optional.of(MediaType.HTML_UTF_8)); validateMessage( capturedContents.get(1), @@ -111,7 +153,7 @@ public class Spec11EmailUtilsTest { Optional.of("my-reply-to@test.com"), "Super Cool Registry Monthly Threat Detector [2018-07-15]", String.format( - emailFormat, + MONTHLY_EMAIL_FORMAT, "b.comMALWAREc.comMALWARE"), Optional.of(MediaType.HTML_UTF_8)); validateMessage( @@ -134,31 +176,13 @@ public class Spec11EmailUtilsTest { // We inspect individual parameters because Message doesn't implement equals(). verify(emailService, times(3)).sendEmail(contentCaptor.capture()); List capturedMessages = contentCaptor.getAllValues(); - String emailFormat = - "Dear registrar partner,

" - + "Super Cool Registry conducts a daily analysis of all domains registered in its TLDs " - + "to identify potential security concerns. On 2018-07-15, the following domains that " - + "your registrar manages were flagged for potential security concerns:

" - + "%s
Domain NameThreat Type

Please communicate " - + "these findings to the registrant and work with the registrant to mitigate any " - + "security issues and have the domains delisted.

Some helpful resources for " - + "getting off a blocked list include:
  • foo

You will continue to " - + "receive daily notices when new domains managed by your registrar are flagged for " - + "abuse, as well as a monthly summary of all of your domains under management that " - + "remain flagged for abuse. Once the registrant has resolved the security issues and " - + "followed the steps to have his or her domain reviewed and delisted it will " - + "automatically be removed from our reporting.

If you would like to change " - + "the email to which these notices are sent please update your abuse contact using " - + "your registrar portal account.

If you have any questions regarding this " - + "notice, please contact my-reply-to@test.com.

"; - validateMessage( capturedMessages.get(0), "my-sender@test.com", "a@fake.com", Optional.of("my-reply-to@test.com"), "Super Cool Registry Daily Threat Detector [2018-07-15]", - String.format(emailFormat, "a.comMALWARE"), + String.format(DAILY_EMAIL_FORMAT, "a.comMALWARE"), Optional.of(MediaType.HTML_UTF_8)); validateMessage( capturedMessages.get(1), @@ -167,7 +191,7 @@ public class Spec11EmailUtilsTest { Optional.of("my-reply-to@test.com"), "Super Cool Registry Daily Threat Detector [2018-07-15]", String.format( - emailFormat, + DAILY_EMAIL_FORMAT, "b.comMALWAREc.comMALWARE"), Optional.of(MediaType.HTML_UTF_8)); validateMessage( @@ -181,8 +205,13 @@ public class Spec11EmailUtilsTest { } @Test - public void testFailure_emailsAlert() throws MessagingException { + public void testOneFailure_sendsAlert() throws Exception { + // If there is one failure, we should still send the other message and then an alert email + LinkedHashSet matches = new LinkedHashSet<>(); + matches.add(getMatchA()); + matches.add(getMatchB()); doThrow(new RuntimeException(new MessagingException("expected"))) + .doNothing() .doNothing() .when(emailService) .sendEmail(contentCaptor.capture()); @@ -191,21 +220,42 @@ public class Spec11EmailUtilsTest { RuntimeException.class, () -> emailUtils.emailSpec11Reports( - date, Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL, "bar", sampleThreatMatches())); - assertThat(thrown).hasMessageThat().isEqualTo("Emailing spec11 report failed"); + date, + Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL, + "Super Cool Registry Monthly Threat Detector [2018-07-15]", + matches)); assertThat(thrown) - .hasCauseThat() .hasMessageThat() - .isEqualTo("javax.mail.MessagingException: expected"); + .isEqualTo("Emailing Spec11 reports failed, first exception:"); + assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("expected"); // Verify we sent an e-mail alert - verify(emailService, times(2)).sendEmail(contentCaptor.capture()); + verify(emailService, times(3)).sendEmail(contentCaptor.capture()); + List capturedMessages = contentCaptor.getAllValues(); validateMessage( - contentCaptor.getValue(), + capturedMessages.get(0), + "my-sender@test.com", + "a@fake.com", + Optional.of("my-reply-to@test.com"), + "Super Cool Registry Monthly Threat Detector [2018-07-15]", + String.format(MONTHLY_EMAIL_FORMAT, "a.comMALWARE"), + Optional.of(MediaType.HTML_UTF_8)); + validateMessage( + capturedMessages.get(1), + "my-sender@test.com", + "b@fake.com", + Optional.of("my-reply-to@test.com"), + "Super Cool Registry Monthly Threat Detector [2018-07-15]", + String.format( + MONTHLY_EMAIL_FORMAT, + "b.comMALWAREc.comMALWARE"), + Optional.of(MediaType.HTML_UTF_8)); + validateMessage( + capturedMessages.get(2), "my-sender@test.com", "my-receiver@test.com", Optional.empty(), "Spec11 Emailing Failure 2018-07-15", - "Emailing spec11 reports failed due to expected", + "Emailing Spec11 reports failed due to expected", Optional.empty()); } @@ -223,36 +273,6 @@ public class Spec11EmailUtilsTest { Optional.empty()); } - @Test - public void testWarning_emptyEmailAddressForRegistrars() throws Exception { - ImmutableSet matchesWithoutEmails = - sampleThreatMatches().stream() - .map(matches -> RegistrarThreatMatches.create("", matches.threatMatches())) - .collect(toImmutableSet()); - emailUtils.emailSpec11Reports( - date, - Spec11EmailSoyInfo.DAILY_SPEC_11_EMAIL, - "Super Cool Registry Daily Threat Detector [2018-07-15]", - matchesWithoutEmails); - verify(emailService).sendEmail(contentCaptor.capture()); - validateMessage( - contentCaptor.getValue(), - "my-sender@test.com", - "my-receiver@test.com", - Optional.empty(), - "Spec11 Pipeline Warning 2018-07-15", - "No errors occurred but the following matches had no associated email: \n" - + "[RegistrarThreatMatches{registrarEmailAddress=, threatMatches=[ThreatMatch" - + "{threatType=MALWARE, platformType=ANY_PLATFORM, metadata=NONE," - + " fullyQualifiedDomainName=a.com}]}, RegistrarThreatMatches{registrarEmailAddress=," - + " threatMatches=[ThreatMatch{threatType=MALWARE, platformType=ANY_PLATFORM," - + " metadata=NONE, fullyQualifiedDomainName=b.com}, ThreatMatch{threatType=MALWARE," - + " platformType=ANY_PLATFORM, metadata=NONE, fullyQualifiedDomainName=c.com}]}]\n\n" - + "This should not occur; please make sure we have email addresses for these" - + " registrar(s).", - Optional.empty()); - } - private void validateMessage( EmailMessage message, String from, diff --git a/javatests/google/registry/reporting/spec11/Spec11RegistrarThreatMatchesParserTest.java b/javatests/google/registry/reporting/spec11/Spec11RegistrarThreatMatchesParserTest.java index c13d72ab8..dc8712102 100644 --- a/javatests/google/registry/reporting/spec11/Spec11RegistrarThreatMatchesParserTest.java +++ b/javatests/google/registry/reporting/spec11/Spec11RegistrarThreatMatchesParserTest.java @@ -78,28 +78,11 @@ public class Spec11RegistrarThreatMatchesParserTest { } /** The expected contents of the sample spec11 report file */ - public static ImmutableSet sampleThreatMatches() throws Exception { + static ImmutableSet sampleThreatMatches() throws Exception { return ImmutableSet.of(getMatchA(), getMatchB()); } - private void setupFile(String fileWithContent, String fileDate) { - GcsFilename gcsFilename = - new GcsFilename( - "test-bucket", - String.format("icann/spec11/2018-07/SPEC11_MONTHLY_REPORT_%s", fileDate)); - when(gcsUtils.existsAndNotEmpty(gcsFilename)).thenReturn(true); - when(gcsUtils.openInputStream(gcsFilename)) - .thenAnswer( - (args) -> - new ByteArrayInputStream( - loadFile(fileWithContent).getBytes(StandardCharsets.UTF_8))); - } - - private static String loadFile(String filename) { - return TestDataHelper.loadFile(Spec11EmailUtils.class, filename); - } - - private static RegistrarThreatMatches getMatchA() throws Exception { + static RegistrarThreatMatches getMatchA() throws Exception { return RegistrarThreatMatches.create( "a@fake.com", ImmutableList.of( @@ -112,7 +95,7 @@ public class Spec11RegistrarThreatMatchesParserTest { "fullyQualifiedDomainName", "a.com"))))); } - private static RegistrarThreatMatches getMatchB() throws Exception { + static RegistrarThreatMatches getMatchB() throws Exception { return RegistrarThreatMatches.create( "b@fake.com", ImmutableList.of( @@ -131,4 +114,21 @@ public class Spec11RegistrarThreatMatchesParserTest { "threatEntryMetadata", "NONE", "fullyQualifiedDomainName", "c.com"))))); } + + private void setupFile(String fileWithContent, String fileDate) { + GcsFilename gcsFilename = + new GcsFilename( + "test-bucket", + String.format("icann/spec11/2018-07/SPEC11_MONTHLY_REPORT_%s", fileDate)); + when(gcsUtils.existsAndNotEmpty(gcsFilename)).thenReturn(true); + when(gcsUtils.openInputStream(gcsFilename)) + .thenAnswer( + (args) -> + new ByteArrayInputStream( + loadFile(fileWithContent).getBytes(StandardCharsets.UTF_8))); + } + + private static String loadFile(String filename) { + return TestDataHelper.loadFile(Spec11EmailUtils.class, filename); + } }