From 3b1986116f31f6947bcd89b4abe866852c1e528e Mon Sep 17 00:00:00 2001 From: Tomas Bures Date: Wed, 26 Dec 2018 09:24:46 +0000 Subject: [PATCH] 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 | 6 +++++- server/config/default.yaml | 6 ------ server/lib/campaign-sender.js | 8 ++++---- server/models/send-configurations.js | 2 +- server/routes/webhooks.js | 15 +++++++++++++-- server/services/verp-server.js | 4 ++-- ..._verp_header_options_in_send_configurations.js | 8 ++++++++ 7 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 server/setup/knex/migrations/20181226090000_verp_header_options_in_send_configurations.js diff --git a/client/src/send-configurations/CUD.js b/client/src/send-configurations/CUD.js index 1ce44785..48e25ff8 100644 --- a/client/src/send-configurations/CUD.js +++ b/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 {

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.

 {mailtrainConfig.verpEnabled ?
- + {verpEnabled && } + {verpEnabled && }
:

VERP bounce handling server is not enabled. Modify your server configuration file and restart server to enable it.

 diff --git a/server/config/default.yaml b/server/config/default.yaml index ec23a74b..027ca5ac 100644 --- a/server/config/default.yaml +++ b/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 diff --git a/server/lib/campaign-sender.js b/server/lib/campaign-sender.js index 368cccd2..a9771db9 100644 --- a/server/lib/campaign-sender.js +++ b/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; }); } diff --git a/server/models/send-configurations.js b/server/models/send-configurations.js index faa3fb11..1acfbc92 100644 --- a/server/models/send-configurations.js +++ b/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)); diff --git a/server/routes/webhooks.js b/server/routes/webhooks.js index 7ea3d533..15e71548 100644 --- a/server/routes/webhooks.js +++ b/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); } } diff --git a/server/services/verp-server.js b/server/services/verp-server.js index 8013a398..9b02aa27 100644 --- a/server/services/verp-server.js +++ b/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); } }; diff --git a/server/setup/knex/migrations/20181226090000_verp_header_options_in_send_configurations.js b/server/setup/knex/migrations/20181226090000_verp_header_options_in_send_configurations.js new file mode 100644 index 00000000..6497756d --- /dev/null +++ b/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() => { +})();