From 452e76ceaf53b65e518b5d419cac12cd261943e8 Mon Sep 17 00:00:00 2001
From: Ere Maijala <ere.maijala@helsinki.fi>
Date: Fri, 23 Feb 2018 20:43:29 +0200
Subject: [PATCH] Mailer improvements (#1112)
- Make it possible to define Reply-To addresses in Mailer's send functions and add a setting to override the sender's address with something SPF-friendly.
---
config/vufind/config.ini | 4 +
module/VuFind/src/VuFind/Mailer/Factory.php | 6 +-
module/VuFind/src/VuFind/Mailer/Mailer.php | 113 +++++++++++++++---
.../src/VuFindTest/Mailer/MailerTest.php | 58 +++++++++
4 files changed, 161 insertions(+), 20 deletions(-)
diff --git a/config/vufind/config.ini b/config/vufind/config.ini
index 2706f63a0d7..6a554e409fc 100644
--- a/config/vufind/config.ini
+++ b/config/vufind/config.ini
@@ -472,6 +472,10 @@ maximum_recipients = 1
; will be sent based on user_email_in_from and default_from above, with the email
; setting from the [Site] section used as a last resort.
disable_from = false
+; From field override. Setting this allows keeping the "from" field in email forms
+; but will only use it as a reply-to address. The address defined here is used as the
+; actual "from" address.
+;override_from = "no-reply@myuniversity.edu"
; Being a special case of mail message, sending record results via SMS ("Text this")
; may be "enabled" or "disabled" ("enabled" by default).
diff --git a/module/VuFind/src/VuFind/Mailer/Factory.php b/module/VuFind/src/VuFind/Mailer/Factory.php
index e18bb596ee7..31a07a9eb80 100644
--- a/module/VuFind/src/VuFind/Mailer/Factory.php
+++ b/module/VuFind/src/VuFind/Mailer/Factory.php
@@ -110,6 +110,10 @@ class Factory implements FactoryInterface
$config = $container->get('VuFind\Config\PluginManager')->get('config');
// Create service:
- return new $requestedName($this->getTransport($config));
+ $class = new $requestedName($this->getTransport($config));
+ if (!empty($config->Mail->override_from)) {
+ $class->setFromAddressOverride($config->Mail->override_from);
+ }
+ return $class;
}
}
diff --git a/module/VuFind/src/VuFind/Mailer/Mailer.php b/module/VuFind/src/VuFind/Mailer/Mailer.php
index 8defb61460e..3d134159b2d 100644
--- a/module/VuFind/src/VuFind/Mailer/Mailer.php
+++ b/module/VuFind/src/VuFind/Mailer/Mailer.php
@@ -60,6 +60,13 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
*/
protected $maxRecipients = 1;
+ /**
+ * "From" address override
+ *
+ * @var string
+ */
+ protected $fromAddressOverride = '';
+
/**
* Constructor
*
@@ -139,26 +146,25 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
* @param string $subject Subject line for message
* @param string $body Message body
* @param string $cc CC recipient (null for none)
+ * @param string|Address|AddressList $replyTo Reply-To address (or delimited
+ * list, null for none)
*
* @throws MailException
* @return void
*/
- public function send($to, $from, $subject, $body, $cc = null)
+ public function send($to, $from, $subject, $body, $cc = null, $replyTo = null)
{
- if ($to instanceof AddressList) {
- $recipients = $to;
- } elseif ($to instanceof Address) {
- $recipients = new AddressList();
- $recipients->add($to);
- } else {
- $recipients = $this->stringToAddressList($to);
- }
+ $recipients = $this->convertToAddressList($to);
+ $replyTo = $this->convertToAddressList($replyTo);
// Validate email addresses:
- if ($this->maxRecipients > 0
- && $this->maxRecipients < count($recipients)
- ) {
- throw new MailException('Too Many Email Recipients');
+ if ($this->maxRecipients > 0) {
+ if ($this->maxRecipients < count($recipients)) {
+ throw new MailException('Too Many Email Recipients');
+ }
+ if ($this->maxRecipients < count($replyTo)) {
+ throw new MailException('Too Many Email Reply-To Addresses');
+ }
}
$validator = new \Zend\Validator\EmailAddress();
if (count($recipients) == 0) {
@@ -169,11 +175,32 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
throw new MailException('Invalid Recipient Email Address');
}
}
+ foreach ($replyTo as $current) {
+ if (!$validator->isValid($current->getEmail())) {
+ throw new MailException('Invalid Reply-To Email Address');
+ }
+ }
$fromEmail = ($from instanceof Address)
? $from->getEmail() : $from;
if (!$validator->isValid($fromEmail)) {
throw new MailException('Invalid Sender Email Address');
}
+
+ if (!empty($this->fromAddressOverride)
+ && $this->fromAddressOverride != $fromEmail
+ ) {
+ $replyTo->add($fromEmail);
+ if (!($from instanceof Address)) {
+ $from = new Address($from);
+ }
+ $name = $from->getName();
+ if (!$name) {
+ list($fromPre) = explode('@', $from->getEmail());
+ $name = $fromPre ? $fromPre : null;
+ }
+ $from = new Address($this->fromAddressOverride, $name);
+ }
+
// Convert all exceptions thrown by mailer into MailException objects:
try {
// Send message
@@ -181,11 +208,13 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
->addFrom($from)
->addTo($recipients)
->setBody($body)
- ->setSubject($subject)
- ->setReplyTo($from);
+ ->setSubject($subject);
if ($cc !== null) {
$message->addCc($cc);
}
+ if ($replyTo) {
+ $message->addReplyTo($replyTo);
+ }
$this->getTransport()->send($message);
} catch (\Exception $e) {
throw new MailException($e->getMessage());
@@ -204,12 +233,14 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
* email templates)
* @param string $subject Subject for email (optional)
* @param string $cc CC recipient (null for none)
+ * @param string|Address|AddressList $replyTo Reply-To address (or delimited
+ * list, null for none)
*
* @throws MailException
* @return void
*/
public function sendLink($to, $from, $msg, $url, $view, $subject = null,
- $cc = null
+ $cc = null, $replyTo = null
) {
if (null === $subject) {
$subject = $this->getDefaultLinkSubject();
@@ -220,7 +251,7 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
'msgUrl' => $url, 'to' => $to, 'from' => $from, 'message' => $msg
]
);
- return $this->send($to, $from, $subject, $body, $cc);
+ return $this->send($to, $from, $subject, $body, $cc, $replyTo);
}
/**
@@ -246,12 +277,14 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
* email templates)
* @param string $subject Subject for email (optional)
* @param string $cc CC recipient (null for none)
+ * @param string|Address|AddressList $replyTo Reply-To address (or
+ * delimited list, null for none)
*
* @throws MailException
* @return void
*/
public function sendRecord($to, $from, $msg, $record, $view, $subject = null,
- $cc = null
+ $cc = null, $replyTo = null
) {
if (null === $subject) {
$subject = $this->getDefaultRecordSubject($record);
@@ -262,7 +295,7 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
'driver' => $record, 'to' => $to, 'from' => $from, 'message' => $msg
]
);
- return $this->send($to, $from, $subject, $body, $cc);
+ return $this->send($to, $from, $subject, $body, $cc, $replyTo);
}
/**
@@ -289,4 +322,46 @@ class Mailer implements \VuFind\I18n\Translator\TranslatorAwareInterface
return $this->translate('Library Catalog Record') . ': '
. $record->getBreadcrumb();
}
+
+ /**
+ * Get the "From" address override value
+ *
+ * @return string
+ */
+ public function getFromAddressOverride()
+ {
+ return $this->fromAddressOverride;
+ }
+
+ /**
+ * Set the "From" address override
+ *
+ * @param string $address "From" address
+ *
+ * @return void
+ */
+ public function setFromAddressOverride($address)
+ {
+ $this->fromAddressOverride = $address;
+ }
+
+ /**
+ * Convert the given addresses to an AddressList object
+ *
+ * @param string|Address|AddressList $addresses Addresses
+ *
+ * @return AddressList
+ */
+ protected function convertToAddressList($addresses)
+ {
+ if ($addresses instanceof AddressList) {
+ $result = $addresses;
+ } elseif ($addresses instanceof Address) {
+ $result = new AddressList();
+ $result->add($addresses);
+ } else {
+ $result = $this->stringToAddressList($addresses ? $addresses : '');
+ }
+ return $result;
+ }
}
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php
index f2db494217d..99e479e324b 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Mailer/MailerTest.php
@@ -125,6 +125,29 @@ class MailerTest extends \VuFindTest\Unit\TestCase
$mailer->send($list, 'from@example.com', 'subject', 'body');
}
+ /**
+ * Test sending an email using a from address override.
+ *
+ * @return void
+ */
+ public function testSendWithFromOverride()
+ {
+ $callback = function ($message) {
+ $fromString = $message->getFrom()->current()->toString();
+ return '<to@example.com>' == $message->getTo()->current()->toString()
+ && '<me@example.com>' == $message->getReplyTo()->current()->toString()
+ && 'me <no-reply@example.com>' == $fromString
+ && 'body' == $message->getBody()
+ && 'subject' == $message->getSubject();
+ };
+ $transport = $this->createMock('Zend\Mail\Transport\TransportInterface');
+ $transport->expects($this->once())->method('send')->with($this->callback($callback));
+ $address = new Address('me@example.com');
+ $mailer = new Mailer($transport);
+ $mailer->setFromAddressOverride('no-reply@example.com');
+ $mailer->send('to@example.com', $address, 'subject', 'body');
+ }
+
/**
* Test bad to address.
*
@@ -140,6 +163,23 @@ class MailerTest extends \VuFindTest\Unit\TestCase
$mailer->send('bad@bad', 'from@example.com', 'subject', 'body');
}
+ /**
+ * Test bad reply-to address.
+ *
+ * @return void
+ *
+ * @expectedException VuFind\Exception\Mail
+ * @expectedExceptionMessage Invalid Reply-To Email Address
+ */
+ public function testBadReplyTo()
+ {
+ $transport = $this->createMock('Zend\Mail\Transport\TransportInterface');
+ $mailer = new Mailer($transport);
+ $mailer->send(
+ 'good@good.com', 'from@example.com', 'subject', 'body', null, 'bad@bad'
+ );
+ }
+
/**
* Test empty to address.
*
@@ -170,6 +210,24 @@ class MailerTest extends \VuFindTest\Unit\TestCase
$mailer->send('one@test.com;two@test.com', 'from@example.com', 'subject', 'body');
}
+ /**
+ * Test that we only accept one reply-to address by default
+ *
+ * @return void
+ *
+ * @expectedException VuFind\Exception\Mail
+ * @expectedExceptionMessage Too Many Email Reply-To Addresses
+ */
+ public function testTooManyReplyToAddresses()
+ {
+ $transport = $this->createMock('Zend\Mail\Transport\TransportInterface');
+ $mailer = new Mailer($transport);
+ $mailer->send(
+ 'one@test.com', 'from@example.com', 'subject', 'body',
+ null, 'one@test.com;two@test.com'
+ );
+ }
+
/**
* Test bad from address.
*
--
GitLab