Fixes in VERP handling.

VERP disable header option moved from config to send configurations.
Some additional logging for VERP.

client/src/send-configurations/CUD.js

@@ -83,6 +83,7 @@ export default class CUD extends Component {
                 this.mailerTypes[data.mailer_type].afterLoad(data);
                 data.verpEnabled = !!data.verp_hostname;
                 data.verp_hostname = data.verp_hostname || '';
+                data.verp_disable_sender_header = data.verpEnabled ? !!data.verp_disable_sender_header : false;
             });
 
         } else {
@@ -100,6 +101,7 @@ export default class CUD extends Component {
                 subject_overridable: false,
                 verpEnabled: false,
                 verp_hostname: '',
+                verp_disable_sender_header: false,
                 x_mailer: '',
                 mailer_type: MailerType.ZONE_MTA,
                 ...this.mailerTypes[MailerType.ZONE_MTA].initData()
@@ -156,6 +158,7 @@ export default class CUD extends Component {
             this.mailerTypes[data.mailer_type].beforeSave(data);
             if (!data.verpEnabled) {
                 data.verp_hostname = null;
+                data.verp_disable_sender_header = false;
             }
         });
 
@@ -229,8 +232,9 @@ export default class CUD extends Component {
                         <Trans i18nKey="verpUsuallyOnlyWorksIfYouAreUsingYourOwn"><p className="text-warning">VERP usually only works if you are using your own SMTP server. Regular relay services (SES, SparkPost, Gmail etc.) tend to remove the VERP address from the message.</p></Trans>
                         {mailtrainConfig.verpEnabled ?
                             <div>
-                                <CheckBox id="verpEnabled" text={t('verpEnabled')}/>
+                                <CheckBox id="verpEnabled" label={t('VERP status')} text={t('Enabled')}/>
                                 {verpEnabled && <InputField id="verp_hostname" label={t('serverHostname')} placeholder={t('theVerpServerHostnameEgBouncesexamplecom')} help={t('verpBounceHandlingServerHostnameThis')}/>}
+                                {verpEnabled && <CheckBox id="verp_disable_sender_header" text={t('Disable sender header')} help={t('With DMARC, the Return-Path and From address must match the same domain. By default we get around this by using the VERP address in the Sender header, with the side effect that some email clients diplay an ugly on behalf of message. You can safely disable this Sender header if you\'re not using DMARC or your VERP hostname is in the same domain as the From address.')}/>}
                             </div>
                             :
                             <Trans i18nKey="verpBounceHandlingServerIsNotEnabled"><p>VERP bounce handling server is not enabled. Modify your server configuration file and restart server to enable it.</p></Trans>

server/config/default.yaml

@@ -123,12 +123,6 @@ verp:
   enabled: false
   port: 25
   host: 0.0.0.0
-  # With DMARC, the Return-Path and From address must match the same domain.
-  # By default we get around this by using the VERP address in the Sender header,
-  # with the side effect that some email clients diplay an ugly on behalf of message.
-  # You can safely disable this Sender header if you're not using DMARC or your
-  # VERP hostname is in the same domain as the From address.
-  # disablesenderheader: true
 
 ldap:
   # enable to use ldap user backend

server/lib/campaign-sender.js

@@ -35,7 +35,7 @@ class CampaignSender {
             fieldsGrouped = await fields.listGroupedTx(tx, list.id);
 
             useVerp = config.verp.enabled && sendConfiguration.verp_hostname;
-            useVerpSenderHeader = this.useVerp && config.verp.disablesenderheader !== true;
+            useVerpSenderHeader = useVerp && !sendConfiguration.verp_disable_sender_header;
 
             subscriptionGrouped = await subscriptions.getByCid(context, list.id, subscriptionCid);
             mergeTags = fields.getMergeTags(fieldsGrouped, subscriptionGrouped);
@@ -94,9 +94,9 @@ class CampaignSender {
                 name: tools.formatMessage(campaign, list, subscriptionGrouped, mergeTags, list.to_name, false),
                 address: subscriptionGrouped.email
             },
-            sender: this.useVerpSenderHeader ? campaignAddress + '@' + sendConfiguration.verp_hostname : false,
+            sender: useVerpSenderHeader ? campaignAddress + '@' + sendConfiguration.verp_hostname : false,
 
-            envelope: this.useVerp ? {
+            envelope: useVerp ? {
                 from: campaignAddress + '@' + sendConfiguration.verp_hostname,
                 to: subscriptionGrouped.email
             } : false,
@@ -183,7 +183,7 @@ class CampaignSender {
             }
 
             this.useVerp = config.verp.enabled && this.sendConfiguration.verp_hostname;
-            this.useVerpSenderHeader = this.useVerp && config.verp.disablesenderheader !== true;
+            this.useVerpSenderHeader = this.useVerp && !this.sendConfiguration.verp_disable_sender_header;
         });
     }
 

server/models/send-configurations.js

@@ -14,7 +14,7 @@ const mailers = require('../lib/mailers');
 const senders = require('../lib/senders');
 const dependencyHelpers = require('../lib/dependency-helpers');
 
-const allowedKeys = new Set(['name', 'description', 'from_email', 'from_email_overridable', 'from_name', 'from_name_overridable', 'reply_to', 'reply_to_overridable', 'subject', 'subject_overridable', 'x_mailer', 'verp_hostname', 'mailer_type', 'mailer_settings', 'namespace']);
+const allowedKeys = new Set(['name', 'description', 'from_email', 'from_email_overridable', 'from_name', 'from_name_overridable', 'reply_to', 'reply_to_overridable', 'subject', 'subject_overridable', 'x_mailer', 'verp_hostname', 'verp_disable_sender_header', 'mailer_type', 'mailer_settings', 'namespace']);
 
 const allowedMailerTypes = new Set(Object.values(MailerType));
 

server/routes/webhooks.js

@@ -13,6 +13,8 @@ const uploads = multer();
 
 
 router.postAsync('/aws', async (req, res) => {
+    console.log(req.body);
+
     if (typeof req.body === 'string') {
         req.body = JSON.parse(req.body);
     }
@@ -70,6 +72,8 @@ router.postAsync('/sparkpost', async (req, res) => {
     const events = [].concat(req.body || []); // This is just a cryptic way getting an array regardless whether req.body is empty, one item, or array
 
     for (const curEvent of events) {
+        console.log(curEvent);
+
         let msys = curEvent && curEvent.msys;
         let evt;
 
@@ -81,6 +85,8 @@ router.postAsync('/sparkpost', async (req, res) => {
             continue;
         }
 
+        log.verbose('Sendgrid', 'Received issue "%s" for message id "%s"', evt.type, evt.campaign_id);
+
         const message = await campaigns.getMessageByCid(evt.campaign_id);
         if (!message) {
             continue;
@@ -119,6 +125,9 @@ router.postAsync('/sendgrid', async (req, res) => {
             continue;
         }
 
+        console.log(evt);
+        log.verbose('Sendgrid', 'Received issue "%s" for message id "%s"', evt.event, evt.campaign_id);
+
         const message = await campaigns.getMessageByCid(evt.campaign_id);
         if (!message) {
             continue;
@@ -153,6 +162,9 @@ router.postAsync('/sendgrid', async (req, res) => {
 router.postAsync('/mailgun', uploads.any(), async (req, res) => {
     const evt = req.body;
 
+    console.log(evt);
+    log.verbose('Mailgun', 'Received issue "%s" for message id "%s"', evt.event, evt.campaign_id);
+
     const message = await campaigns.getMessageByCid([].concat(evt && evt.campaign_id || []).shift());
     if (message) {
         switch (evt.event) {
@@ -187,11 +199,10 @@ router.postAsync('/zone-mta', async (req, res) => {
 
         if (req.body.id) {
             const message = await campaigns.getMessageByResponseId(req.body.id);
-            console.log(message);
 
             if (message) {
                 await campaigns.changeStatusByMessage(contextHelpers.getAdminContext(), message, SubscriptionStatus.BOUNCED, true);
-                log.verbose('ZoneMTA', 'Marked message %s as bounced', req.body.id);
+                log.verbose('ZoneMTA', 'Marked message (campaign:%s, list:%s, subscription:%s) as bounced', message.campaign, message.list, message.subscription);
             }
         }
 

server/services/verp-server.js

@@ -32,7 +32,7 @@ async function onRcptTo(address, session) {
 
     session.message = message;
 
-    log.verbose('VERP', 'Incoming message for Campaign %s, List %s, Subscription %s', message.campaign, message.list, message.subscription);
+    log.verbose('VERP', 'Incoming message for campaign:%s, list:%s, subscription:%s', message.campaign, message.list, message.subscription);
 }
 
 function onData(stream, session, callback) {
@@ -56,7 +56,7 @@ function onData(stream, session, callback) {
             return 'Message accepted';
         } else {
             await campaigns.changeStatusByMessage(contextHelpers.getAdminContext(), session.message, SubscriptionStatus.BOUNCED, bounceResult.action === 'failed');
-            log.verbose('VERP', 'Marked message %s as unsubscribed', session.message.campaign);
+            log.verbose('VERP', 'Marked message (campaign:%s, list:%s, subscription:%s) as unsubscribed', session.message.campaign, session.message.list, session.message.subscription);
         }
     };
 

server/setup/knex/migrations/20181226090000_verp_header_options_in_send_configurations.js

@@ -0,0 +1,8 @@
+exports.up = (knex, Promise) => (async() => {
+    await knex.schema.table('send_configurations', table => {
+        table.boolean('verp_disable_sender_header').defaultTo(false);
+    });
+})();
+
+exports.down = (knex, Promise) => (async() => {
+})();