Merge "Check for inbound emails that the change is visible"
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 2b4482a..3a35d80 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -232,7 +232,10 @@
throws UpdateException, RestApiException {
try (ManualRequestContext ctx = oneOffRequestContext.openAs(sender)) {
List<ChangeData> changeDataList =
- queryProvider.get().byLegacyChangeId(Change.id(metadata.changeNumber));
+ queryProvider
+ .get()
+ .enforceVisibility(true)
+ .byLegacyChangeId(Change.id(metadata.changeNumber));
if (changeDataList.isEmpty()) {
sendRejectionEmail(message, InboundEmailRejectionSender.Error.CHANGE_NOT_FOUND);
return;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index b86aa43..85c0212 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -16,6 +16,8 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
@@ -28,7 +30,9 @@
import com.google.common.collect.Streams;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.EmailHeader;
+import com.google.gerrit.entities.Permission;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
@@ -44,6 +48,7 @@
import com.google.gerrit.mail.MailMessage;
import com.google.gerrit.mail.MailProcessingUtil;
import com.google.gerrit.server.mail.receive.MailProcessor;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gerrit.testing.TestCommentHelper;
import com.google.inject.Inject;
@@ -61,6 +66,7 @@
@Inject private MailProcessor mailProcessor;
@Inject private AccountOperations accountOperations;
@Inject private TestCommentHelper testCommentHelper;
+ @Inject private ProjectOperations projectOperations;
private static final CommentValidator mockCommentValidator = mock(CommentValidator.class);
@@ -276,6 +282,69 @@
}
@Test
+ public void sendNotificationOnProjectNotFound() throws Exception {
+ String ts =
+ MailProcessingUtil.rfcDateformatter.format(
+ ZonedDateTime.ofInstant(TimeUtil.now(), ZoneId.of("UTC")));
+
+ String changeUrl = canonicalWebUrl.get() + "c/non-existing-project/+/123";
+
+ // Build Message
+ String txt = newPlaintextBody(changeUrl + "/1", "Test Message", null, null);
+ MailMessage.Builder b =
+ messageBuilderWithDefaultFields()
+ .from(user.getNameEmail())
+ .textContent(txt + textFooterForChange(123, ts));
+
+ sender.clear();
+ mailProcessor.process(b.build());
+
+ assertNotifyTo(user);
+ Message message = sender.nextMessage();
+ assertThat(message.body())
+ .contains(
+ "Gerrit Code Review was unable to process your email because the change was not"
+ + " found.");
+ assertThat(message.headers()).containsKey("Subject");
+ }
+
+ @Test
+ public void sendNotificationOnProjectNotVisible() throws Exception {
+ String changeId = createChangeWithReview();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+
+ String ts =
+ MailProcessingUtil.rfcDateformatter.format(
+ ZonedDateTime.ofInstant(
+ gApi.changes().id(changeId).get().updated.toInstant(), ZoneId.of("UTC")));
+
+ // Block read permissions on the project.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ // Build Message
+ String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null);
+ MailMessage.Builder b =
+ messageBuilderWithDefaultFields()
+ .from(user.getNameEmail())
+ .textContent(txt + textFooterForChange(changeInfo._number, ts));
+
+ sender.clear();
+ mailProcessor.process(b.build());
+
+ assertNotifyTo(user);
+ Message message = sender.nextMessage();
+ assertThat(message.body())
+ .contains(
+ "Gerrit Code Review was unable to process your email because the change was not"
+ + " found.");
+ assertThat(message.headers()).containsKey("Subject");
+ }
+
+ @Test
public void sendNotificationOnChangeNotFound() throws Exception {
String changeId = createChangeWithReview();
ChangeInfo changeInfo = gApi.changes().id(changeId).get();
@@ -303,7 +372,39 @@
assertThat(message.body())
.contains(
"Gerrit Code Review was unable to process your email because the change was not"
- + " found.\nMaybe the change got deleted?");
+ + " found.");
+ assertThat(message.headers()).containsKey("Subject");
+ }
+
+ @Test
+ public void sendNotificationOnChangeNotVisible() throws Exception {
+ String changeId = createChangeWithReview();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+
+ String ts =
+ MailProcessingUtil.rfcDateformatter.format(
+ ZonedDateTime.ofInstant(
+ gApi.changes().id(changeId).get().updated.toInstant(), ZoneId.of("UTC")));
+
+ // Make change private so that it's no visible to user.
+ gApi.changes().id(changeId).setPrivate(true);
+
+ // Build Message
+ String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null);
+ MailMessage.Builder b =
+ messageBuilderWithDefaultFields()
+ .from(user.getNameEmail())
+ .textContent(txt + textFooterForChange(changeInfo._number, ts));
+
+ sender.clear();
+ mailProcessor.process(b.build());
+
+ assertNotifyTo(user);
+ Message message = sender.nextMessage();
+ assertThat(message.body())
+ .contains(
+ "Gerrit Code Review was unable to process your email because the change was not"
+ + " found.");
assertThat(message.headers()).containsKey("Subject");
}
diff --git a/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy b/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy
index 1b50efb..a9e0a44 100644
--- a/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy
+++ b/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy
@@ -68,7 +68,8 @@
{template InboundEmailRejection_CHANGE_NOT_FOUND kind="text"}
Gerrit Code Review was unable to process your email because the change was not found.
{\n}
- Maybe the change got deleted?
+ Maybe the project doesn't exist or is not visible? Maybe the change is not visible or got
+ deleted?
{call InboundEmailRejectionFooter /}
{/template}
diff --git a/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy b/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy
index 9662497..3444b7f 100644
--- a/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy
+++ b/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy
@@ -87,7 +87,8 @@
Gerrit Code Review was unable to process your email because the change was not found.
</p>
<p>
- Maybe the change got deleted?
+ Maybe the project doesn't exist or is not visible? Maybe the change is not visible or got
+ deleted?
<p>
{call InboundEmailRejectionFooterHtml /}
{/template}
