JAMES-2346 Comply with the RFC-8098 toward determining a MDN recipient The Disposition-Notification-To field should be used. It should then be checked against the Return-Path to avoid mail bombing.
Note that MDN generation had been extracted from processor for easy unit testing logic. Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/a11ce53c Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/a11ce53c Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/a11ce53c Branch: refs/heads/master Commit: a11ce53c06ad97534334562fc583dab7b73d1294 Parents: af5a472 Author: benwa <btell...@linagora.com> Authored: Fri Mar 9 11:29:58 2018 +0700 Committer: benwa <btell...@linagora.com> Committed: Tue Mar 13 15:11:54 2018 +0700 ---------------------------------------------------------------------- .../methods/integration/SendMDNMethodTest.java | 71 +++++++++++++++++ .../InvalidOriginMessageForMDNException.java | 38 ++++++++++ .../james/jmap/methods/SendMDNProcessor.java | 55 +++++--------- .../org/apache/james/jmap/model/JmapMDN.java | 80 ++++++++++++++++++++ .../apache/james/jmap/model/JmapMDNTest.java | 60 ++++++++++++--- 5 files changed, 257 insertions(+), 47 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/a11ce53c/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SendMDNMethodTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SendMDNMethodTest.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SendMDNMethodTest.java index 6e72316..fbe6f58 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SendMDNMethodTest.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SendMDNMethodTest.java @@ -105,6 +105,39 @@ public abstract class SendMDNMethodTest { " \"setMessages\"," + " {" + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"headers\":{\"Disposition-Notification-To\":\"" + BOB + "\"}," + + " \"from\": { \"name\": \"Bob\", \"email\": \"" + BOB + "\"}," + + " \"to\": [{ \"name\": \"User\", \"email\": \"" + USERNAME + "\"}]," + + " \"subject\": \"Message with an attachment\"," + + " \"textBody\": \"Test body, plain text version\"," + + " \"htmlBody\": \"Test <b>body</b>, HTML version\"," + + " \"mailboxIds\": [\"" + outboxId + "\"] " + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + with() + .header("Authorization", bobAccessToken.serialize()) + .body(requestBody) + .post("/jmap") + .then() + .extract() + .body() + .path(ARGUMENTS + ".created." + messageCreationId + ".id"); + + calmlyAwait.until(() -> !getMessageIdListForAccount(accessToken.serialize()).isEmpty()); + } + + private void sendAWrongInitialMessage() { + String messageCreationId = "creationId"; + String outboxId = getOutboxId(bobAccessToken); + String requestBody = "[" + + " [" + + " \"setMessages\"," + + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + " \"from\": { \"name\": \"Bob\", \"email\": \"" + BOB + "\"}," + " \"to\": [{ \"name\": \"User\", \"email\": \"" + USERNAME + "\"}]," + " \"subject\": \"Message with an attachment\"," + @@ -210,6 +243,44 @@ public abstract class SendMDNMethodTest { } @Test + public void sendMDNShouldFailOnInvalidMessages() { + sendAWrongInitialMessage(); + List<String> messageIds = getMessageIdListForAccount(accessToken.serialize()); + + String creationId = "creation-1"; + + given() + .header("Authorization", accessToken.serialize()) + .body("[[\"setMessages\", {\"sendMDN\": {" + + "\"" + creationId + "\":{" + + " \"messageId\":\"" + messageIds.get(0) + "\"," + + " \"subject\":\"subject\"," + + " \"textBody\":\"textBody\"," + + " \"reportingUA\":\"reportingUA\"," + + " \"disposition\":{" + + " \"actionMode\":\"automatic-action\","+ + " \"sendingMode\":\"MDN-sent-automatically\","+ + " \"type\":\"processed\""+ + " }" + + "}" + + "}}, \"#0\"]]") + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".MDNNotSent", hasEntry( + equalTo(creationId), + hasEntry("type", "invalidArgument"))) + .body(ARGUMENTS + ".MDNNotSent", hasEntry( + equalTo(creationId), + hasEntry("description", "Origin messageId '" + messageIds.get(0) + "' is invalid. " + + "A Message Delivery Notification can not be generated for it. " + + "Explanation: Disposition-Notification-To header is missing"))); + } + + @Test public void sendMDNShouldSendAMDNBackToTheOriginalMessageAuthor() { sendAnInitialMessage(); http://git-wip-us.apache.org/repos/asf/james-project/blob/a11ce53c/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/InvalidOriginMessageForMDNException.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/InvalidOriginMessageForMDNException.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/InvalidOriginMessageForMDNException.java new file mode 100644 index 0000000..3cdd8a1 --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/exceptions/InvalidOriginMessageForMDNException.java @@ -0,0 +1,38 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you 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 org.apache.james.jmap.exceptions; + +public class InvalidOriginMessageForMDNException extends Exception { + private static final String MISSING_HEADER = " header is missing"; + + public static InvalidOriginMessageForMDNException missingHeader(String headerName) { + return new InvalidOriginMessageForMDNException(headerName + MISSING_HEADER); + } + + private final String explanation; + + public InvalidOriginMessageForMDNException(String explanation) { + this.explanation = explanation; + } + + public String getExplanation() { + return explanation; + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/a11ce53c/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SendMDNProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SendMDNProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SendMDNProcessor.java index 5f13d44..6142ef0 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SendMDNProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SendMDNProcessor.java @@ -29,11 +29,10 @@ import javax.inject.Inject; import javax.mail.Flags; import javax.mail.MessagingException; -import org.apache.james.core.User; +import org.apache.james.jmap.exceptions.InvalidOriginMessageForMDNException; import org.apache.james.jmap.exceptions.MessageNotFoundException; import org.apache.james.jmap.model.Envelope; import org.apache.james.jmap.model.JmapMDN; -import org.apache.james.jmap.model.MDNDisposition; import org.apache.james.jmap.model.MessageFactory; import org.apache.james.jmap.model.SetError; import org.apache.james.jmap.model.SetMessagesRequest; @@ -51,14 +50,12 @@ import org.apache.james.mailbox.model.MessageId; import org.apache.james.mailbox.model.MessageResult; import org.apache.james.mdn.MDN; import org.apache.james.mdn.MDNReport; -import org.apache.james.mdn.fields.Disposition; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.mime4j.codec.DecodeMonitor; import org.apache.james.mime4j.dom.Message; import org.apache.james.mime4j.dom.field.ParseException; import org.apache.james.mime4j.message.DefaultMessageBuilder; import org.apache.james.mime4j.stream.MimeConfig; -import org.apache.james.mime4j.util.MimeUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -109,6 +106,17 @@ public class SendMDNProcessor implements SetMessagesProcessor { MessageId messageId = sendMdn(MDNCreationEntry, mailboxSession); return SetMessagesResponse.builder() .mdnSent(MDNCreationEntry.getCreationId(), messageId); + } catch (InvalidOriginMessageForMDNException e) { + return SetMessagesResponse.builder() + .mdnNotSent(MDNCreationEntry.getCreationId(), + SetError.builder() + .description(String.format("Origin messageId '%s' is invalid." + + " A Message Delivery Notification can not be generated for it." + + " Explanation: " + e.getExplanation(), + MDNCreationEntry.getValue().getMessageId().serialize())) + .type("invalidArgument") + .build()); + } catch (MessageNotFoundException e) { return SetMessagesResponse.builder() .mdnNotSent(MDNCreationEntry.getCreationId(), @@ -129,14 +137,15 @@ public class SendMDNProcessor implements SetMessagesProcessor { } } - private MessageId sendMdn(ValueWithId.MDNCreationEntry MDNCreationEntry, MailboxSession mailboxSession) throws MailboxException, IOException, MessagingException, ParseException, MessageNotFoundException { + private MessageId sendMdn(ValueWithId.MDNCreationEntry MDNCreationEntry, MailboxSession mailboxSession) + throws MailboxException, IOException, MessagingException, ParseException, MessageNotFoundException, InvalidOriginMessageForMDNException { + JmapMDN mdn = MDNCreationEntry.getValue(); Message originalMessage = retrieveOriginalMessage(mdn, mailboxSession); - MDNReport mdnReport = generateReport(mdn, originalMessage, mailboxSession); + MDNReport mdnReport = mdn.generateReport(originalMessage, mailboxSession); List<MessageAttachment> reportAsAttachment = ImmutableList.of(convertReportToAttachment(mdnReport)); - User user = User.fromUsername(mailboxSession.getUser().getUserName()); - Message mdnAnswer = generateMDNMessage(originalMessage, mdn, mdnReport, user); + Message mdnAnswer = mdn.generateMDNMessage(originalMessage, mailboxSession); Flags seen = new Flags(Flags.Flag.SEEN); MessageFactory.MetaDataWithContent metaDataWithContent = messageAppender.appendMessageInMailbox(mdnAnswer, @@ -148,19 +157,6 @@ public class SendMDNProcessor implements SetMessagesProcessor { return metaDataWithContent.getMessageId(); } - private Message generateMDNMessage(Message originalMessage, JmapMDN mdn, MDNReport mdnReport, User user) throws ParseException, IOException { - return MDN.builder() - .report(mdnReport) - .humanReadableText(mdn.getTextBody()) - .build() - .asMime4JMessageBuilder() - .setTo(originalMessage.getSender().getAddress()) - .setFrom(user.asString()) - .setSubject(mdn.getSubject()) - .setMessageId(MimeUtil.createUniqueMessageId(user.getDomainPart().orElse(null))) - .build(); - } - private Message retrieveOriginalMessage(JmapMDN mdn, MailboxSession mailboxSession) throws MailboxException, IOException, MessageNotFoundException { List<MessageResult> messages = messageIdManager.getMessages(ImmutableList.of(mdn.getMessageId()), FetchGroupImpl.HEADERS, @@ -188,23 +184,6 @@ public class SendMDNProcessor implements SetMessagesProcessor { .build(); } - private MDNReport generateReport(JmapMDN mdn, Message originalMessage, MailboxSession mailboxSession) { - return MDNReport.builder() - .dispositionField(generateDisposition(mdn.getDisposition())) - .originalRecipientField(mailboxSession.getUser().getUserName()) - .originalMessageIdField(originalMessage.getMessageId()) - .finalRecipientField(mailboxSession.getUser().getUserName()) - .reportingUserAgentField(mdn.getReportingUA()) - .build(); - } - - private Disposition generateDisposition(MDNDisposition disposition) { - return Disposition.builder() - .actionMode(disposition.getActionMode()) - .sendingMode(disposition.getSendingMode()) - .type(disposition.getType()) - .build(); - } private MessageManager getOutbox(MailboxSession mailboxSession) throws MailboxException { return systemMailboxesProvider.getMailboxByRole(Role.OUTBOX, mailboxSession) http://git-wip-us.apache.org/repos/asf/james-project/blob/a11ce53c/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/JmapMDN.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/JmapMDN.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/JmapMDN.java index 0e1df56..7178c1e 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/JmapMDN.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/JmapMDN.java @@ -19,19 +19,41 @@ package org.apache.james.jmap.model; +import java.io.IOException; import java.util.Objects; +import java.util.Optional; +import java.util.stream.Stream; +import org.apache.james.core.User; +import org.apache.james.jmap.exceptions.InvalidOriginMessageForMDNException; +import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.model.MessageId; +import org.apache.james.mdn.MDN; +import org.apache.james.mdn.MDNReport; +import org.apache.james.mdn.fields.Disposition; +import org.apache.james.mime4j.codec.DecodeMonitor; +import org.apache.james.mime4j.dom.Message; +import org.apache.james.mime4j.dom.address.AddressList; +import org.apache.james.mime4j.dom.address.Mailbox; +import org.apache.james.mime4j.dom.address.MailboxList; +import org.apache.james.mime4j.dom.field.AddressListField; +import org.apache.james.mime4j.dom.field.ParseException; +import org.apache.james.mime4j.field.AddressListFieldLenientImpl; +import org.apache.james.mime4j.util.MimeUtil; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; @JsonDeserialize(builder = JmapMDN.Builder.class) public class JmapMDN { + public static final String DISPOSITION_NOTIFICATION_TO = "Disposition-Notification-To"; + public static final String RETURN_PATH = "Return-Path"; + public static Builder builder() { return new Builder(); } @@ -116,6 +138,64 @@ public class JmapMDN { return disposition; } + public Message generateMDNMessage(Message originalMessage, MailboxSession mailboxSession) throws ParseException, IOException, InvalidOriginMessageForMDNException { + + User user = User.fromUsername(mailboxSession.getUser().getUserName()); + + return MDN.builder() + .report(generateReport(originalMessage, mailboxSession)) + .humanReadableText(textBody) + .build() + .asMime4JMessageBuilder() + .setTo(getSenderAddress(originalMessage)) + .setFrom(user.asString()) + .setSubject(subject) + .setMessageId(MimeUtil.createUniqueMessageId(user.getDomainPart().orElse(null))) + .build(); + } + + private String getSenderAddress(Message originalMessage) throws InvalidOriginMessageForMDNException { + return getAddressForHeader(originalMessage, DISPOSITION_NOTIFICATION_TO) + .orElseThrow(() -> InvalidOriginMessageForMDNException.missingHeader(DISPOSITION_NOTIFICATION_TO)) + .getAddress(); + } + + private Optional<Mailbox> getAddressForHeader(Message originalMessage, String fieldName) { + return Optional.ofNullable(originalMessage.getHeader() + .getFields(fieldName)) + .orElse(ImmutableList.of()) + .stream() + .map(field -> AddressListFieldLenientImpl.PARSER.parse(field, new DecodeMonitor())) + .findFirst() + .map(AddressListField::getAddressList) + .map(AddressList::flatten) + .map(MailboxList::stream) + .orElse(Stream.of()) + .findFirst(); + } + + + public MDNReport generateReport(Message originalMessage, MailboxSession mailboxSession) throws InvalidOriginMessageForMDNException { + if (originalMessage.getMessageId() == null) { + throw InvalidOriginMessageForMDNException.missingHeader("Message-ID"); + } + return MDNReport.builder() + .dispositionField(generateDisposition()) + .originalRecipientField(mailboxSession.getUser().getUserName()) + .originalMessageIdField(originalMessage.getMessageId()) + .finalRecipientField(mailboxSession.getUser().getUserName()) + .reportingUserAgentField(getReportingUA()) + .build(); + } + + private Disposition generateDisposition() { + return Disposition.builder() + .actionMode(disposition.getActionMode()) + .sendingMode(disposition.getSendingMode()) + .type(disposition.getType()) + .build(); + } + @Override public final boolean equals(Object o) { if (o instanceof JmapMDN) { http://git-wip-us.apache.org/repos/asf/james-project/blob/a11ce53c/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/JmapMDNTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/JmapMDNTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/JmapMDNTest.java index 5cb045d..e692d87 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/JmapMDNTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/JmapMDNTest.java @@ -22,10 +22,17 @@ package org.apache.james.jmap.model; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.nio.charset.StandardCharsets; + +import org.apache.james.jmap.exceptions.InvalidOriginMessageForMDNException; +import org.apache.james.mailbox.mock.MockMailboxSession; import org.apache.james.mailbox.model.TestMessageId; import org.apache.james.mdn.action.mode.DispositionActionMode; import org.apache.james.mdn.sending.mode.DispositionSendingMode; import org.apache.james.mdn.type.DispositionType; +import org.apache.james.mime4j.dom.Message; +import org.apache.james.mime4j.dom.address.Mailbox; +import org.apache.james.mime4j.stream.RawField; import org.junit.Test; import nl.jqno.equalsverifier.EqualsVerifier; @@ -35,12 +42,20 @@ public class JmapMDNTest { public static final String TEXT_BODY = "text body"; public static final String SUBJECT = "subject"; public static final String REPORTING_UA = "reportingUA"; + public static final TestMessageId MESSAGE_ID = TestMessageId.of(45); public static final MDNDisposition DISPOSITION = MDNDisposition.builder() .actionMode(DispositionActionMode.Automatic) .sendingMode(DispositionSendingMode.Automatic) .type(DispositionType.Processed) .build(); - public static final TestMessageId MESSAGE_ID = TestMessageId.of(45); + public static final JmapMDN MDN = JmapMDN.builder() + .disposition(DISPOSITION) + .messageId(MESSAGE_ID) + .reportingUA(REPORTING_UA) + .subject(SUBJECT) + .textBody(TEXT_BODY) + .build(); + public static final MockMailboxSession MAILBOX_SESSION = new MockMailboxSession("u...@localhost.com"); @Test public void shouldMatchBeanContract() { @@ -51,14 +66,7 @@ public class JmapMDNTest { @Test public void builderShouldReturnObjectWhenAllFieldsAreValid() { - assertThat( - JmapMDN.builder() - .disposition(DISPOSITION) - .messageId(MESSAGE_ID) - .reportingUA(REPORTING_UA) - .subject(SUBJECT) - .textBody(TEXT_BODY) - .build()) + assertThat(MDN) .isEqualTo(new JmapMDN(MESSAGE_ID, SUBJECT, TEXT_BODY, REPORTING_UA, DISPOSITION)); } @@ -122,4 +130,38 @@ public class JmapMDNTest { .isInstanceOf(IllegalStateException.class); } + @Test + public void generateMDNMessageShouldUseDispositionHeaders() throws Exception { + String senderAddress = "sender@local"; + Message originMessage = Message.Builder.of() + .setMessageId("45...@local.com") + .setFrom(senderAddress) + .setBody("body", StandardCharsets.UTF_8) + .addField(new RawField(JmapMDN.RETURN_PATH, "<" + senderAddress + ">")) + .addField(new RawField(JmapMDN.DISPOSITION_NOTIFICATION_TO, "<" + senderAddress + ">")) + .build(); + + assertThat( + MDN.generateMDNMessage(originMessage, MAILBOX_SESSION) + .getTo()) + .extracting(address -> (Mailbox) address) + .extracting(Mailbox::getAddress) + .containsExactly(senderAddress); + } + + @Test + public void generateMDNMessageShouldFailOnMissingDisposition() throws Exception { + String senderAddress = "sender@local"; + Message originMessage = Message.Builder.of() + .setMessageId("45...@local.com") + .setFrom(senderAddress) + .setBody("body", StandardCharsets.UTF_8) + .addField(new RawField(JmapMDN.RETURN_PATH, "<" + senderAddress + ">")) + .build(); + + assertThatThrownBy(() -> + MDN.generateMDNMessage(originMessage, MAILBOX_SESSION)) + .isInstanceOf(InvalidOriginMessageForMDNException.class); + } + } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org