Merge "Implement ListMailFilter with black and whitelist functionality"

Documentation/config-gerrit.txt

@@ -3538,6 +3538,27 @@ immediately and will not have a fetch delay.
 +
 Defaults to false.
 
+[[receiveemail.filter.mode]]receiveemail.filter.mode::
++
+A black- and whitelist filter to filter incoming emails.
++
+If `OFF`, emails are not filtered by the list filter.
++
+If `WHITELIST`, only emails where a pattern from
+<<receiveemail.filter.patterns,receiveemail.filter.patterns>>
+matches 'From' will be processed.
++
+If `BLACKLIST`, only emails where no pattern from
+<<receiveemail.filter.patterns,receiveemail.filter.patterns>>
+matches 'From' will be processed.
++
+Defaults to `OFF`.
+
+[[receiveemail.filter.patterns]]receiveemail.filter.patterns::
++
+A list of regular expressions to match the email sender against. This can also
+be a list of addresses when regular expression characters are escaped.
+
 [[sendemail]]
 === Section sendemail
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java

@@ -0,0 +1,141 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// 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 com.google.gerrit.acceptance.server.mail;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
+import com.google.gerrit.extensions.client.Comment;
+import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.server.mail.receive.MailMessage;
+import java.util.HashMap;
+import org.joda.time.DateTime;
+import org.junit.Ignore;
+
+@Ignore
+public class AbstractMailIT extends AbstractDaemonTest {
+
+  protected MailMessage.Builder messageBuilderWithDefaultFields() {
+    MailMessage.Builder b = MailMessage.builder();
+    b.id("some id");
+    b.from(user.emailAddress);
+    b.addTo(user.emailAddress); // Not evaluated
+    b.subject("");
+    b.dateReceived(new DateTime());
+    return b;
+  }
+
+  protected String createChangeWithReview() throws Exception {
+    // Create change
+    String file = "gerrit-server/test.txt";
+    String contents = "contents \nlorem \nipsum \nlorem";
+    PushOneCommit push =
+        pushFactory.create(db, admin.getIdent(), testRepo, "first subject", file, contents);
+    PushOneCommit.Result r = push.to("refs/for/master");
+    String changeId = r.getChangeId();
+
+    // Review it
+    ReviewInput input = new ReviewInput();
+    input.message = "I have two comments";
+    input.comments = new HashMap<>();
+    CommentInput c1 = newComment(file, Side.REVISION, 0, "comment on file");
+    CommentInput c2 = newComment(file, Side.REVISION, 2, "inline comment");
+    input.comments.put(c1.path, ImmutableList.of(c1, c2));
+    revision(r).review(input);
+    return changeId;
+  }
+
+  protected static CommentInput newComment(String path, Side side, int line, String message) {
+    CommentInput c = new CommentInput();
+    c.path = path;
+    c.side = side;
+    c.line = line != 0 ? line : null;
+    c.message = message;
+    if (line != 0) {
+      Comment.Range range = new Comment.Range();
+      range.startLine = line;
+      range.startCharacter = 1;
+      range.endLine = line;
+      range.endCharacter = 5;
+      c.range = range;
+    }
+    return c;
+  }
+
+  /**
+   * Create a plaintext message body with the specified comments.
+   *
+   * @param changeMessage
+   * @param c1 Comment in reply to first inline comment.
+   * @param f1 Comment on file one.
+   * @param fc1 Comment in reply to a comment of file 1.
+   * @return A string with all inline comments and the original quoted email.
+   */
+  protected static String newPlaintextBody(
+      String changeURL, String changeMessage, String c1, String f1, String fc1) {
+    return (changeMessage == null ? "" : changeMessage + "\n")
+        + "> Foo Bar has posted comments on this change. (  \n"
+        + "> "
+        + changeURL
+        + " )\n"
+        + "> \n"
+        + "> Change subject: Test change\n"
+        + "> ...............................................................\n"
+        + "> \n"
+        + "> \n"
+        + "> Patch Set 1: Code-Review+1\n"
+        + "> \n"
+        + "> (3 comments)\n"
+        + "> \n"
+        + "> "
+        + changeURL
+        + "/gerrit-server/test.txt\n"
+        + "> File  \n"
+        + "> gerrit-server/test.txt:\n"
+        + (f1 == null ? "" : f1 + "\n")
+        + "> \n"
+        + "> Patch Set #4:\n"
+        + "> "
+        + changeURL
+        + "/gerrit-server/test.txt\n"
+        + "> \n"
+        + "> Some comment"
+        + "> \n"
+        + (fc1 == null ? "" : fc1 + "\n")
+        + "> "
+        + changeURL
+        + "/gerrit-server/test.txt@2\n"
+        + "> PS1, Line 2: throw new Exception(\"Object has unsupported: \" +\n"
+        + ">               :             entry.getValue() +\n"
+        + ">               :             \" must be java.util.Date\");\n"
+        + "> Should entry.getKey() be included in this message?\n"
+        + "> \n"
+        + (c1 == null ? "" : c1 + "\n")
+        + "> \n";
+  }
+
+  protected static String textFooterForChange(String changeId, String timestamp) {
+    return "Gerrit-Change-Id: "
+        + changeId
+        + "\n"
+        + "Gerrit-PatchSet: 1\n"
+        + "Gerrit-MessageType: comment\n"
+        + "Gerrit-Comment-Date: "
+        + timestamp
+        + "\n";
+  }
+}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ListMailFilterIT.java

@@ -0,0 +1,121 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// 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 com.google.gerrit.acceptance.server.mail;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.GerritConfig;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.server.mail.MailUtil;
+import com.google.gerrit.server.mail.receive.MailMessage;
+import com.google.gerrit.server.mail.receive.MailProcessor;
+import com.google.inject.Inject;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+
+@NoHttpd
+public class ListMailFilterIT extends AbstractMailIT {
+  @Inject private MailProcessor mailProcessor;
+
+  @Test
+  @GerritConfig(name = "receiveemail.filter.mode", value = "OFF")
+  public void listFilterOff() throws Exception {
+    ChangeInfo changeInfo = createChangeAndReplyByEmail();
+    // Check that the comments from the email have been persisted
+    Collection<ChangeMessageInfo> messages = gApi.changes().id(changeInfo.id).get().messages;
+    assertThat(messages).hasSize(3);
+  }
+
+  @Test
+  @GerritConfig(name = "receiveemail.filter.mode", value = "WHITELIST")
+  @GerritConfig(
+    name = "receiveemail.filter.patterns",
+    values = {".+ser@example\\.com", "a@b\\.com"}
+  )
+  public void listFilterWhitelistDoesNotFilterListedUser() throws Exception {
+    ChangeInfo changeInfo = createChangeAndReplyByEmail();
+    // Check that the comments from the email have been persisted
+    Collection<ChangeMessageInfo> messages = gApi.changes().id(changeInfo.id).get().messages;
+    assertThat(messages).hasSize(3);
+  }
+
+  @Test
+  @GerritConfig(name = "receiveemail.filter.mode", value = "WHITELIST")
+  @GerritConfig(
+    name = "receiveemail.filter.patterns",
+    values = {".+@gerritcodereview\\.com", "a@b\\.com"}
+  )
+  public void listFilterWhitelistFiltersNotListedUser() throws Exception {
+    ChangeInfo changeInfo = createChangeAndReplyByEmail();
+    // Check that the comments from the email have NOT been persisted
+    Collection<ChangeMessageInfo> messages = gApi.changes().id(changeInfo.id).get().messages;
+    assertThat(messages).hasSize(2);
+  }
+
+  @Test
+  @GerritConfig(name = "receiveemail.filter.mode", value = "BLACKLIST")
+  @GerritConfig(
+    name = "receiveemail.filter.patterns",
+    values = {".+@gerritcodereview\\.com", "a@b\\.com"}
+  )
+  public void listFilterBlacklistDoesNotFilterNotListedUser() throws Exception {
+    ChangeInfo changeInfo = createChangeAndReplyByEmail();
+    // Check that the comments from the email have been persisted
+    Collection<ChangeMessageInfo> messages = gApi.changes().id(changeInfo.id).get().messages;
+    assertThat(messages).hasSize(3);
+  }
+
+  @Test
+  @GerritConfig(name = "receiveemail.filter.mode", value = "BLACKLIST")
+  @GerritConfig(
+    name = "receiveemail.filter.patterns",
+    values = {".+@example\\.com", "a@b\\.com"}
+  )
+  public void listFilterBlacklistFiltersListedUser() throws Exception {
+    ChangeInfo changeInfo = createChangeAndReplyByEmail();
+    // Check that the comments from the email have been persisted
+    Collection<ChangeMessageInfo> messages = gApi.changes().id(changeInfo.id).get().messages;
+    assertThat(messages).hasSize(2);
+  }
+
+  private ChangeInfo createChangeAndReplyByEmail() throws Exception {
+    String changeId = createChangeWithReview();
+    ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+    List<CommentInfo> comments = gApi.changes().id(changeId).current().commentsAsList();
+    String ts =
+        MailUtil.rfcDateformatter.format(
+            ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
+
+    // Build Message
+    MailMessage.Builder b = messageBuilderWithDefaultFields();
+    String txt =
+        newPlaintextBody(
+            canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
+            "Test Message",
+            null,
+            null,
+            null);
+    b.textContent(txt + textFooterForChange(changeId, ts));
+
+    mailProcessor.process(b.build());
+    return changeInfo;
+  }
+}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java

@@ -16,18 +16,10 @@ package com.google.gerrit.acceptance.server.mail;
 
 import static com.google.common.truth.Truth.assertThat;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
-import com.google.gerrit.extensions.client.Comment;
-import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
 import com.google.gerrit.extensions.common.CommentInfo;
-import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.mail.MailUtil;
 import com.google.gerrit.server.mail.receive.MailMessage;
 import com.google.gerrit.server.mail.receive.MailProcessor;
@@ -35,12 +27,10 @@ import com.google.inject.Inject;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
-import org.joda.time.DateTime;
 import org.junit.Test;
 
-public class MailProcessorIT extends AbstractDaemonTest {
+public class MailProcessorIT extends AbstractMailIT {
   @Inject private MailProcessor mailProcessor;
 
   @Test
@@ -236,115 +226,4 @@ public class MailProcessorIT extends AbstractDaemonTest {
 
     assertNotifyTo(admin);
   }
-
-  private static CommentInput newComment(String path, Side side, int line, String message) {
-    CommentInput c = new CommentInput();
-    c.path = path;
-    c.side = side;
-    c.line = line != 0 ? line : null;
-    c.message = message;
-    if (line != 0) {
-      Comment.Range range = new Comment.Range();
-      range.startLine = line;
-      range.startCharacter = 1;
-      range.endLine = line;
-      range.endCharacter = 5;
-      c.range = range;
-    }
-    return c;
-  }
-
-  /**
-   * Create a plaintext message body with the specified comments.
-   *
-   * @param changeMessage
-   * @param c1 Comment in reply to first inline comment.
-   * @param f1 Comment on file one.
-   * @param fc1 Comment in reply to a comment of file 1.
-   * @return A string with all inline comments and the original quoted email.
-   */
-  private static String newPlaintextBody(
-      String changeURL, String changeMessage, String c1, String f1, String fc1) {
-    return (changeMessage == null ? "" : changeMessage + "\n")
-        + "> Foo Bar has posted comments on this change. (  \n"
-        + "> "
-        + changeURL
-        + " )\n"
-        + "> \n"
-        + "> Change subject: Test change\n"
-        + "> ...............................................................\n"
-        + "> \n"
-        + "> \n"
-        + "> Patch Set 1: Code-Review+1\n"
-        + "> \n"
-        + "> (3 comments)\n"
-        + "> \n"
-        + "> "
-        + changeURL
-        + "/gerrit-server/test.txt\n"
-        + "> File  \n"
-        + "> gerrit-server/test.txt:\n"
-        + (f1 == null ? "" : f1 + "\n")
-        + "> \n"
-        + "> Patch Set #4:\n"
-        + "> "
-        + changeURL
-        + "/gerrit-server/test.txt\n"
-        + "> \n"
-        + "> Some comment"
-        + "> \n"
-        + (fc1 == null ? "" : fc1 + "\n")
-        + "> "
-        + changeURL
-        + "/gerrit-server/test.txt@2\n"
-        + "> PS1, Line 2: throw new Exception(\"Object has unsupported: \" +\n"
-        + ">               :             entry.getValue() +\n"
-        + ">               :             \" must be java.util.Date\");\n"
-        + "> Should entry.getKey() be included in this message?\n"
-        + "> \n"
-        + (c1 == null ? "" : c1 + "\n")
-        + "> \n";
-  }
-
-  private static String textFooterForChange(String changeId, String timestamp) {
-    return "Gerrit-Change-Id: "
-        + changeId
-        + "\n"
-        + "Gerrit-PatchSet: 1\n"
-        + "Gerrit-MessageType: comment\n"
-        + "Gerrit-Comment-Date: "
-        + timestamp
-        + " \n";
-  }
-
-  private MailMessage.Builder messageBuilderWithDefaultFields() {
-    MailMessage.Builder b = MailMessage.builder();
-    b.id("some id");
-    Address address = new Address(user.fullName, user.email);
-    b.from(address);
-    b.addTo(address);
-    b.subject("");
-    b.dateReceived(new DateTime());
-    return b;
-  }
-
-  private String createChangeWithReview() throws Exception {
-    // Create change
-    String file = "gerrit-server/test.txt";
-    String contents = "contents \nlorem \nipsum \nlorem";
-    PushOneCommit push =
-        pushFactory.create(db, admin.getIdent(), testRepo, "first subject", file, contents);
-    PushOneCommit.Result r = push.to("refs/for/master");
-    String changeId = r.getChangeId();
-
-    // Review it
-    ReviewInput input = new ReviewInput();
-    input.message = "I have two comments";
-    input.comments = new HashMap<>();
-    CommentInput c1 = newComment(file, Side.REVISION, 0, "comment on file");
-    CommentInput c2 = newComment(file, Side.REVISION, 2, "inline comment");
-    input.comments.put(c1.path, ImmutableList.of(c1, c2));
-    revision(r).review(input);
-    return changeId;
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -20,6 +20,7 @@ import com.google.common.cache.Cache;
 import com.google.gerrit.audit.AuditModule;
 import com.google.gerrit.common.EventListener;
 import com.google.gerrit.common.UserScopedEventListener;
+import com.google.gerrit.extensions.annotations.Exports;
 import com.google.gerrit.extensions.api.changes.ActionVisitor;
 import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
 import com.google.gerrit.extensions.auth.oauth.OAuthLoginProvider;
@@ -133,6 +134,7 @@ import com.google.gerrit.server.git.validators.UploadValidators;
 import com.google.gerrit.server.group.GroupModule;
 import com.google.gerrit.server.index.change.ReindexAfterUpdate;
 import com.google.gerrit.server.mail.EmailModule;
+import com.google.gerrit.server.mail.ListMailFilter;
 import com.google.gerrit.server.mail.MailFilter;
 import com.google.gerrit.server.mail.send.AddKeySender;
 import com.google.gerrit.server.mail.send.AddReviewerSender;
@@ -351,7 +353,6 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicMap.mapOf(binder(), DownloadCommand.class);
     DynamicMap.mapOf(binder(), CloneCommand.class);
     DynamicMap.mapOf(binder(), ReviewerSuggestion.class);
-    DynamicMap.mapOf(binder(), MailFilter.class);
     DynamicSet.setOf(binder(), ExternalIncludedIn.class);
     DynamicMap.mapOf(binder(), ProjectConfigEntry.class);
     DynamicSet.setOf(binder(), PatchSetWebLink.class);
@@ -369,6 +370,9 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicSet.setOf(binder(), AssigneeValidationListener.class);
     DynamicSet.setOf(binder(), ActionVisitor.class);
 
+    DynamicMap.mapOf(binder(), MailFilter.class);
+    bind(MailFilter.class).annotatedWith(Exports.named("ListMailFilter")).to(ListMailFilter.class);
+
     factory(UploadValidators.Factory.class);
     DynamicSet.setOf(binder(), UploadValidationListener.class);
 

gerrit-server/src/main/java/com/google/gerrit/server/mail/ListMailFilter.java

@@ -0,0 +1,62 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// 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 com.google.gerrit.server.mail;
+
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.mail.receive.MailMessage;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.Arrays;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.lib.Config;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class ListMailFilter implements MailFilter {
+  public enum ListFilterMode {
+    OFF,
+    WHITELIST,
+    BLACKLIST
+  }
+
+  private static final Logger log = LoggerFactory.getLogger(ListMailFilter.class);
+
+  private final ListFilterMode mode;
+  private final Pattern mailPattern;
+
+  @Inject
+  ListMailFilter(@GerritServerConfig Config cfg) {
+    this.mode = cfg.getEnum("receiveemail", "filter", "mode", ListFilterMode.OFF);
+    String[] addresses = cfg.getStringList("receiveemail", "filter", "patterns");
+    String concat = Arrays.asList(addresses).stream().collect(Collectors.joining("|"));
+    this.mailPattern = Pattern.compile(concat);
+  }
+
+  @Override
+  public boolean shouldProcessMessage(MailMessage message) {
+    if (mode == ListFilterMode.OFF) {
+      return true;
+    }
+
+    boolean match = mailPattern.matcher(message.from().email).find();
+    if (mode == ListFilterMode.WHITELIST && !match || mode == ListFilterMode.BLACKLIST && match) {
+      log.info("Mail message from " + message.from() + " rejected by list filter");
+      return false;
+    }
+    return true;
+  }
+}