From ab3d83dabefc9421245ceae7d53df5e9448547ea Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Fri, 14 Jun 2019 14:30:44 -0400 Subject: [PATCH] Filter out non-published domains in Spec11 emails --- .../reporting/spec11/Spec11EmailUtils.java | 32 +++++++--- .../spec11/Spec11EmailUtilsTest.java | 61 ++++++++++++++++++- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java b/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java index 77d5ca034..01574fa9c 100644 --- a/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java +++ b/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java @@ -17,16 +17,20 @@ package google.registry.reporting.spec11; import static com.google.common.base.Throwables.getRootCause; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.io.Resources.getResource; +import static google.registry.model.ofy.ObjectifyService.ofy; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; import com.google.template.soy.SoyFileSet; import com.google.template.soy.parseinfo.SoyTemplateInfo; import com.google.template.soy.tofu.SoyTofu; import com.google.template.soy.tofu.SoyTofu.Renderer; +import google.registry.beam.spec11.ThreatMatch; import google.registry.config.RegistryConfig.Config; +import google.registry.model.domain.DomainBase; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; @@ -34,7 +38,6 @@ import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; import java.util.List; import java.util.Map; -import java.util.Set; import javax.inject.Inject; import javax.mail.MessagingException; import javax.mail.internet.InternetAddress; @@ -84,16 +87,19 @@ public class Spec11EmailUtils { LocalDate date, SoyTemplateInfo soyTemplateInfo, String subject, - Set registrarThreatMatchesSet) { + ImmutableSet registrarThreatMatchesSet) { 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)); + RegistrarThreatMatches filteredMatches = filterOutNonPublishedMatches(registrarThreatMatches); + if (!filteredMatches.threatMatches().isEmpty()) { + try { + // Handle exceptions individually per registrar so that one failed email doesn't prevent + // the rest from being sent. + emailRegistrar(date, soyTemplateInfo, subject, filteredMatches); + } catch (Throwable e) { + failedMatchesBuilder.put(registrarThreatMatches, getRootCause(e)); + } } } ImmutableMap failedMatches = failedMatchesBuilder.build(); @@ -120,6 +126,16 @@ public class Spec11EmailUtils { "Spec11 reporting completed successfully."); } + private RegistrarThreatMatches filterOutNonPublishedMatches( + RegistrarThreatMatches registrarThreatMatches) { + ImmutableList filteredMatches = registrarThreatMatches.threatMatches().stream() + .filter(threatMatch -> + ofy().load().type(DomainBase.class).filter("fullyQualifiedDomainName", + threatMatch.fullyQualifiedDomainName()).first().now().shouldPublishToDns() + ).collect(toImmutableList()); + return RegistrarThreatMatches.create(registrarThreatMatches.clientId(), filteredMatches); + } + private void emailRegistrar( LocalDate date, SoyTemplateInfo soyTemplateInfo, diff --git a/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java b/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java index 39414bae1..1b0ba4dd7 100644 --- a/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java +++ b/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java @@ -15,9 +15,15 @@ package google.registry.reporting.spec11; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.eppcommon.StatusValue.CLIENT_HOLD; +import static google.registry.model.eppcommon.StatusValue.SERVER_HOLD; +import static google.registry.model.ofy.ObjectifyService.ofy; 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.DatastoreHelper.createTld; +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.JUnitBackports.assertThrows; import static org.mockito.Mockito.doThrow; @@ -29,6 +35,9 @@ 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 com.googlecode.objectify.Key; +import google.registry.model.domain.DomainBase; +import google.registry.model.host.HostResource; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; import google.registry.testing.AppEngineRule; import google.registry.util.EmailMessage; @@ -118,6 +127,9 @@ public class Spec11EmailUtilsTest { private ArgumentCaptor contentCaptor; private final LocalDate date = new LocalDate(2018, 7, 15); + private DomainBase aDomain; + private DomainBase bDomain; + @Before public void setUp() throws Exception { emailService = mock(SendEmailService.class); @@ -132,6 +144,12 @@ public class Spec11EmailUtilsTest { new InternetAddress("my-reply-to@test.com"), FAKE_RESOURCES, "Super Cool Registry"); + + createTld("com"); + HostResource host = persistActiveHost("ns1.example.com"); + aDomain = persistDomainWithHost("a.com", host); + bDomain = persistDomainWithHost("b.com", host); + persistDomainWithHost("c.com", host); } @Test @@ -210,6 +228,41 @@ public class Spec11EmailUtilsTest { Optional.empty()); } + @Test + public void testSuccess_skipsInactiveDomain() throws Exception { + // CLIENT_HOLD and SERVER_HOLD mean no DNS so we don't need to email it out + persistResource( + ofy().load().entity(aDomain).now().asBuilder().addStatusValue(SERVER_HOLD) + .build()); + persistResource( + ofy().load().entity(bDomain).now().asBuilder().addStatusValue(CLIENT_HOLD) + .build()); + 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(emailService, times(2)).sendEmail(contentCaptor.capture()); + List capturedContents = contentCaptor.getAllValues(); + validateMessage( + capturedContents.get(0), + "my-sender@test.com", + "new.registrar@example.com", + Optional.of("my-reply-to@test.com"), + "Super Cool Registry Monthly Threat Detector [2018-07-15]", + String.format(MONTHLY_EMAIL_FORMAT, "c.comMALWARE"), + Optional.of(MediaType.HTML_UTF_8)); + validateMessage( + capturedContents.get(1), + "my-sender@test.com", + "my-receiver@test.com", + Optional.empty(), + "Spec11 Pipeline Success 2018-07-15", + "Spec11 reporting completed successfully.", + Optional.empty()); + } + @Test public void testOneFailure_sendsAlert() throws Exception { // If there is one failure, we should still send the other message and then an alert email @@ -229,7 +282,7 @@ public class Spec11EmailUtilsTest { date, Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL, "Super Cool Registry Monthly Threat Detector [2018-07-15]", - matches)); + ImmutableSet.copyOf(matches))); assertThat(thrown) .hasMessageThat() .isEqualTo("Emailing Spec11 reports failed, first exception:"); @@ -340,4 +393,10 @@ public class Spec11EmailUtilsTest { contentType.ifPresent(expectedContentBuilder::setContentType); assertThat(message).isEqualTo(expectedContentBuilder.build()); } + + private static DomainBase persistDomainWithHost(String domainName, HostResource host) { + return persistResource( + newDomainBase(domainName).asBuilder().setNameservers(ImmutableSet.of(Key.create(host))) + .build()); + } }