Skip to content

Commit

Permalink
feat(mail): enable UTF-8 mail address following RFC 6530 IQSS#7424
Browse files Browse the repository at this point in the history
- Make support configurable using new setting, defaulting to true
  (most MTAs today should support SMTPUTF8)
- If need be and an admin disables the support, make email validator
  deny UTF-8 chars (otherwise no mails could be sent!)
- Add logging message to send method to give hint about necessary
  UTF-8 support everywhere in the chain
- Add (extensible) integration test for MailServiceBean to check
  sending mails actually works
  • Loading branch information
poikilotherm committed Oct 10, 2023
1 parent 9c7d9b5 commit b970eb5
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public boolean sendSystemEmail(String to, String subject, String messageText, bo
return true;
} catch (MessagingException ae) {
logger.log(Level.WARNING, "Failed to send mail to %s: %s".formatted(to, ae.getMessage()), ae);
logger.info("When UTF-8 characters in recipients: make sure MTA supports it and JVM option " + JvmSettings.MAIL_MTA_SUPPORT_UTF8.getScopedKey() + "=true");
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public enum JvmSettings {
MAIL_MTA_AUTH(SCOPE_MAIL_MTA, "auth"),
MAIL_MTA_USER(SCOPE_MAIL_MTA, "user"),
MAIL_MTA_PASSWORD(SCOPE_MAIL_MTA, "password"),
MAIL_MTA_SUPPORT_UTF8(SCOPE_MAIL_MTA, "allow-utf8-addresses"),
// Placeholder setting for a large list of extra settings
MAIL_MTA_SETTING(SCOPE_MAIL_MTA),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ Properties getMailProperties() {
// See https://jakarta.ee/specifications/mail/2.1/apidocs/jakarta.mail/jakarta/mail/package-summary
configuration.put("mail.transport.protocol", "smtp");
configuration.put("mail.debug", JvmSettings.MAIL_DEBUG.lookupOptional(Boolean.class).orElse(false).toString());
// Only enable if your MTA properly supports UTF-8 mail addresses following RFC 6530/6531/6532.
// Before, we used a hack to put the raw UTF-8 mail address into the system.
// Now, make it proper, but make it possible to disable it - see also EMailValidator.
// Default = true from microprofile-config.properties as most MTAs these days support SMTPUTF8 extension
configuration.put("mail.mime.allowutf8", JvmSettings.MAIL_MTA_SUPPORT_UTF8.lookup(Boolean.class).toString());

configuration.put(PREFIX + "host", JvmSettings.MAIL_MTA_HOST.lookup());
// default = false from microprofile-config.properties
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
package edu.harvard.iq.dataverse.validation;

import edu.harvard.iq.dataverse.settings.JvmSettings;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;

import org.apache.commons.validator.routines.EmailValidator;

import java.nio.charset.StandardCharsets;

/**
*
* @author skraffmi
*/
public class EMailValidator implements ConstraintValidator<ValidateEmail, String> {

private static final boolean MTA_SUPPORTS_UTF8 = JvmSettings.MAIL_MTA_SUPPORT_UTF8.lookup(Boolean.class);

@Override
public boolean isValid(String value, ConstraintValidatorContext context) {
return isEmailValid(value);
Expand All @@ -23,6 +28,10 @@ public boolean isValid(String value, ConstraintValidatorContext context) {
* @return true when valid, false when invalid (null = valid!)
*/
public static boolean isEmailValid(String value) {
return value == null || EmailValidator.getInstance().isValid(value);
return value == null || (EmailValidator.getInstance().isValid(value) &&
// If the MTA isn't able to handle UTF-8 mail addresses following RFC 6530/6531/6532, we can only declare
// mail addresses using 7bit ASCII (RFC 821) as valid.
// Beyond scope for Apache Commons Validator, see also https://issues.apache.org/jira/browse/VALIDATOR-487
(StandardCharsets.US_ASCII.newEncoder().canEncode(value) || MTA_SUPPORTS_UTF8) );
}
}
1 change: 1 addition & 0 deletions src/main/resources/META-INF/microprofile-config.properties
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ dataverse.rserve.tempdir=/tmp/Rserv

# MAIL
dataverse.mail.mta.auth=false
dataverse.mail.mta.allow-utf8-addresses=true
# In containers, default to hostname smtp, a container on the same network
%ct.dataverse.mail.mta.host=smtp

Expand Down
143 changes: 143 additions & 0 deletions src/test/java/edu/harvard/iq/dataverse/MailServiceBeanIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package edu.harvard.iq.dataverse;

import edu.harvard.iq.dataverse.branding.BrandingUtil;
import edu.harvard.iq.dataverse.settings.JvmSettings;
import edu.harvard.iq.dataverse.settings.SettingsServiceBean;
import edu.harvard.iq.dataverse.util.MailSessionProducer;
import edu.harvard.iq.dataverse.util.testing.JvmSetting;
import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings;
import edu.harvard.iq.dataverse.util.testing.Tags;
import io.restassured.RestAssured;
import jakarta.mail.Session;
import jakarta.mail.internet.AddressException;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.containers.wait.strategy.Wait;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.util.List;

import static io.restassured.RestAssured.given;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* An integration test using a fake SMTP MTA to check for outgoing mails.
* LIMITATION: This test cannot possibly check if the production and injection of the session via CDI
* works, as it is not running within a servlet container. This would require usage of Arquillian
* or and end-to-end API test with a deployed application.
*/

@Tag(Tags.INTEGRATION_TEST)
@Tag(Tags.USES_TESTCONTAINERS)
@Testcontainers(disabledWithoutDocker = true)
@ExtendWith(MockitoExtension.class)
@LocalJvmSettings
@JvmSetting(key = JvmSettings.MAIL_MTA_HOST, method = "tcSmtpHost")
@JvmSetting(key = JvmSettings.MAIL_MTA_SETTING, method = "tcSmtpPort", varArgs = "port")
class MailServiceBeanIT {

private static final Integer PORT_SMTP = 1025;
private static final Integer PORT_HTTP = 1080;

static MailServiceBean mailer;
static Session session;
static SettingsServiceBean settingsServiceBean = Mockito.mock(SettingsServiceBean.class);
static DataverseServiceBean dataverseServiceBean = Mockito.mock(DataverseServiceBean.class);

@BeforeAll
static void setUp() {
// Setup mocks behavior, inject as deps
Mockito.when(settingsServiceBean.getValueForKey(SettingsServiceBean.Key.SystemEmail)).thenReturn("noreply@example.org");
BrandingUtil.injectServices(dataverseServiceBean, settingsServiceBean);

// Must happen here, as we need Testcontainers to start the container first...
session = new MailSessionProducer().getSession();
mailer = new MailServiceBean(session, settingsServiceBean);
}

/*
Cannot use maildev/maildev here. Also MailCatcher doesn't provide official support for SMTPUTF8.
Also maildev does advertise the feature and everything is fine over the wire, both JSON API and Web UI
of maildev have an encoding problem - UTF-8 mail addresses following RFC 6530/6531/6532 are botched.
Neither MailCatcher nor MailHog have this problem, yet the API of MailCatcher is much simpler
to use during testing, which is why we are going with it.
*/
@Container
static GenericContainer<?> maildev = new GenericContainer<>("dockage/mailcatcher")
.withExposedPorts(PORT_HTTP, PORT_SMTP)
.waitingFor(Wait.forHttp("/"));

static String tcSmtpHost() {
return maildev.getHost();
}

static String tcSmtpPort() {
return maildev.getMappedPort(PORT_SMTP).toString();
}

@BeforeAll
static void setup() {
RestAssured.baseURI = "http://" + tcSmtpHost();
RestAssured.port = maildev.getMappedPort(PORT_HTTP);
}

static List<String> mailTo() {
return List.of(
"pete@mailinator.com", // one example using ASCII only, make sure it works
"michélle.pereboom@example.com",
"begüm.vriezen@example.com",
"lótus.gonçalves@example.com",
"lótus.gonçalves@éxample.com",
"begüm.vriezen@example.cologne",
"رونیکا.محمدخان@example.com",
"lótus.gonçalves@example.cóm"
);
}

@ParameterizedTest
@MethodSource("mailTo")
@JvmSetting(key = JvmSettings.MAIL_MTA_SUPPORT_UTF8, value = "true")
void sendEmailIncludingUTF8(String mailAddress) {
given().when().get("/messages")
.then()
.statusCode(200);

// given
Session session = new MailSessionProducer().getSession();
MailServiceBean mailer = new MailServiceBean(session, settingsServiceBean);

// when
boolean sent = mailer.sendSystemEmail(mailAddress, "Test", "Test üüü", false);

// then
assertTrue(sent);
//RestAssured.get("/messages").body().prettyPrint();
given().when().get("/messages")
.then()
.statusCode(200)
.body("last().recipients.first()", equalTo("<" + mailAddress + ">"));
}

@Test
@JvmSetting(key = JvmSettings.SYSTEM_EMAIL, value = "test@example.org")
@JvmSetting(key = JvmSettings.MAIL_MTA_SUPPORT_UTF8, value = "false")
void mailRejectedWhenUTF8AddressButNoSupport() throws AddressException {
// given
Session session = new MailSessionProducer().getSession();
MailServiceBean mailer = new MailServiceBean(session, settingsServiceBean);
String to = "michélle.pereboom@example.com";

assertFalse(mailer.sendSystemEmail(to, "Test", "Test", false));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,22 @@
import edu.harvard.iq.dataverse.util.testing.JvmSetting;
import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings;
import jakarta.mail.internet.AddressException;
import jakarta.mail.internet.InternetAddress;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

import jakarta.mail.internet.InternetAddress;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

import java.io.UnsupportedEncodingException;

import static edu.harvard.iq.dataverse.settings.SettingsServiceBean.*;
import static edu.harvard.iq.dataverse.settings.SettingsServiceBean.Key;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -145,7 +144,6 @@ class SendSystemMail {
void skipIfNoSystemAddress() {
assertFalse(mailServiceBean.sendSystemEmail("target@example.org", "Test", "Test", false));
}

}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package edu.harvard.iq.dataverse.validation;

import edu.harvard.iq.dataverse.settings.JvmSettings;
import edu.harvard.iq.dataverse.util.testing.JvmSetting;
import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -14,7 +17,8 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class EMailValidatorTest {
@LocalJvmSettings
class EMailValidatorTest {

private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();

Expand Down Expand Up @@ -82,4 +86,27 @@ public void testConstraint(boolean expected, String mail) {
violations.stream().findFirst().ifPresent( c -> {
assertTrue(c.getMessage().contains(mail)); });
}

public static Stream<Arguments> emailAsciiUtf8Examples() {
return Stream.of(
Arguments.of("false", "pete@mailinator.com"),
Arguments.of("false", "foobar@mail.science"),
Arguments.of("true", "lótus.gonçalves@éxample.com"),
Arguments.of("true", "begüm.vriezen@example.cologne")
);
}

@ParameterizedTest
@MethodSource("emailAsciiUtf8Examples")
@JvmSetting(key = JvmSettings.MAIL_MTA_SUPPORT_UTF8, value = "false")
void validateWhenMTADoesNotSupportUTF8(boolean needsUTF8Support, String mail) {
assertEquals(!needsUTF8Support, EMailValidator.isEmailValid(mail));
}

@ParameterizedTest
@MethodSource("emailAsciiUtf8Examples")
@JvmSetting(key = JvmSettings.MAIL_MTA_SUPPORT_UTF8, value = "true")
void validateWhenMTASupportsUTF8(boolean needsUTF8Support, String mail) {
assertTrue(EMailValidator.isEmailValid(mail));
}
}

0 comments on commit b970eb5

Please sign in to comment.