From 32e2e61789128cecccafb84006dd794e6de05fca Mon Sep 17 00:00:00 2001 From: Tomas Bures Date: Sun, 30 Apr 2017 13:01:22 -0400 Subject: [PATCH] Unsubscription is identified by subscriber cid. This effectivelly allows only the recipient of the email to unsubscribe. This addresses issue #221. I also scraped the "auto" parameter which automatically submits the unsubscription form when the link is clicked in a campaign email. Instead, I introduced the unsubscription options ONE_STEP, ONE_STEP_WITH_FORM, TWO_STEP, TWO_STEP_WITH_FORM. The options without "_WITH_FORM" shall behave like when called with "auto". This functionality is to come. Currently it behaves as ONE_STEP_WITH_FORM. --- lib/models/lists.js | 8 +- lib/models/subscriptions.js | 227 ++++++------------ lib/tools.js | 2 +- routes/lists.js | 12 + routes/subscription.js | 168 +++---------- services/sender.js | 2 +- .../subscription-unsubscribe-form.hbs | 11 +- views/subscription/web-unsubscribe.mjml.hbs | 3 - 8 files changed, 126 insertions(+), 307 deletions(-) diff --git a/lib/models/lists.js b/lib/models/lists.js index 7ee7dcc6..839fefe1 100644 --- a/lib/models/lists.js +++ b/lib/models/lists.js @@ -9,9 +9,11 @@ let tableHelpers = require('../table-helpers'); const UnsubscriptionMode = { ONE_STEP: 0, - TWO_STEP: 1, - MANUAL: 2, - MAX: 3 + ONE_STEP_WITH_FORM: 1, + TWO_STEP: 2, + TWO_STEP_WITH_FORM: 3, + MANUAL: 4, + MAX: 5 }; module.exports.UnsubscriptionMode = UnsubscriptionMode; diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js index 57169257..24158ffd 100644 --- a/lib/models/subscriptions.js +++ b/lib/models/subscriptions.js @@ -107,84 +107,20 @@ module.exports.addConfirmation = (list, email, ip, data, callback) => { return callback(null, false); } - fields.list(list.id, (err, fieldList) => { - if (err) { - return callback(err); - } + if (data._skip) { + log.info('Subscription', 'Confirmation message for %s marked to be skipped (%s)', email, JSON.stringify(data)); + return callback(null, cid); + } - let encryptionKeys = []; - fields.getRow(fieldList, data).forEach(field => { - if (field.type === 'gpg' && field.value) { - encryptionKeys.push(field.value.trim()); - } - }); - - settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl'], (err, configItems) => { - if (err) { - return callback(err); - } - - setImmediate(() => { - if (data._skip) { - log.info('Subscription', 'Confirmation message for %s marked to be skipped (%s)', email, JSON.stringify(data)); - return; - } - - // FIXME - move to router - const mailOpts = { - subject: _('%s: Please Confirm Subscription'), - confirmUrlRoute: '/subscription/confirm/', - templateType: 'subscription' - }; - - let sendMail = (html, text) => { - mailer.sendMail({ - from: { - name: configItems.defaultFrom, - address: configItems.defaultAddress - }, - to: { - name: [].concat(data.firstName || []).concat(data.lastName || []).join(' '), - address: email - }, - subject: util.format(mailOpts.subject, list.name), - encryptionKeys - }, { - html, - text, - data: { - title: list.name, - contactAddress: configItems.defaultAddress, - defaultPostaddress: configItems.defaultPostaddress, - confirmUrl: urllib.resolve(configItems.serviceUrl, mailOpts.confirmUrlRoute + cid) - } - }, err => { - if (err) { - log.error('Subscription', err); - } - }); - }; - - let text = { - template: 'subscription/mail-confirm-' + mailOpts.templateType + '-text.hbs' - }; - - let html = { - template: 'subscription/mail-confirm-' + mailOpts.templateType + '-html.mjml.hbs', - layout: 'subscription/layout.mjml.hbs', - type: 'mjml' - }; - - helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { - if (err) { - return sendMail(html, text); - } - - sendMail(tmpl.html, tmpl.text); - }); - }); - return callback(null, cid); - }); + // FIXME - customize from the router + const mailOpts = { + ignoreDisableConfirmations: true + }; + const relativeUrls = { + confirmUrl: '/subscription/confirm/' + cid + }; + module.exports.sendMail(list, email, 'confirm-subscription', _('%s: Please Confirm Subscription'), relativeUrls, mailOpts, data, (err) => { + return callback(err, cid); }); }); }); @@ -395,8 +331,6 @@ module.exports.insert = (listId, meta, subscription, callback) => { queryArgs = values.concat(existing.id); query = 'UPDATE `subscription__' + listId + '` SET ' + keys.map(key => '`' + key + '`=?') + ' WHERE id=? LIMIT 1'; } - console.log(query); - console.log(queryArgs); connection.query(query, queryArgs, (err, result) => { if (err) { @@ -647,9 +581,8 @@ module.exports.update = (listId, cid, updates, allowEmail, callback) => { }); }; -module.exports.unsubscribe = (listId, email, campaignId, callback) => { +module.exports.unsubscribe = (listId, subscriberCid, campaignId, callback) => { listId = Number(listId) || 0; - email = (email || '').toString().trim(); campaignId = (campaignId || '').toString().trim() || false; @@ -657,8 +590,8 @@ module.exports.unsubscribe = (listId, email, campaignId, callback) => { return callback(new Error(_('Missing List ID'))); } - if (!email) { - return callback(new Error(_('Missing email address'))); + if (!subscriberCid) { + return callback(new Error(_('Missing subscriber cid'))); } db.getConnection((err, connection) => { @@ -666,7 +599,7 @@ module.exports.unsubscribe = (listId, email, campaignId, callback) => { return callback(err); } - connection.query('SELECT * FROM `subscription__' + listId + '` WHERE `email`=?', [email], (err, rows) => { + connection.query('SELECT * FROM `subscription__' + listId + '` WHERE `cid`=?', [subscriberCid], (err, rows) => { connection.release(); if (err) { return callback(err); @@ -1166,92 +1099,86 @@ module.exports.updateAddress = (list, cid, updates, ip, callback) => { }; -module.exports.sendMail = (listId, email, template, subject, mailOpts, subscription, callback) => { +module.exports.sendMail = (list, email, template, subject, relativeUrls, mailOpts, subscription, callback) => { db.getConnection((err, connection) => { if (err) { return callback(err); } - lists.get(listId, (err, list) => { - if (!err && !list) { - err = new Error(_('Selected list not found')); - err.status = 404; - } - + fields.list(list.id, (err, fieldList) => { if (err) { - return next(err); + return callback(err); } - fields.list(list.id, (err, fieldList) => { + let encryptionKeys = []; + fields.getRow(fieldList, subscription).forEach(field => { + if (field.type === 'gpg' && field.value) { + encryptionKeys.push(field.value.trim()); + } + }); + + settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl', 'disableConfirmations'], (err, configItems) => { if (err) { return callback(err); } - let encryptionKeys = []; - fields.getRow(fieldList, subscription).forEach(field => { - if (field.type === 'gpg' && field.value) { - encryptionKeys.push(field.value.trim()); - } - }); + if (!mailOpts.ignoreDisableConfirmations && configItems.disableConfirmations) { + return; + } - settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl'], (err, configItems) => { - if (err) { - return callback(err); - } + const data = { + title: list.name, + homepage: configItems.defaultHomepage || configItems.serviceUrl, + contactAddress: configItems.defaultAddress, + defaultPostaddress: configItems.defaultPostaddress, + }; - const data = { - title: list.name, - contactAddress: configItems.defaultAddress, - defaultPostaddress: configItems.defaultPostaddress, - }; + for (let relativeUrlKey in relativeUrls) { + data[relativeUrlKey] = urllib.resolve(configItems.serviceUrl, relativeUrls[relativeUrlKey]); + } - if (mailOpts.confirmUrlRoute) { - data.confirmUrl = urllib.resolve(configItems.serviceUrl, mailOpts.confirmUrlRoute + cid) - } - - function sendMail(html, text) { - mailer.sendMail({ - from: { - name: configItems.defaultFrom, - address: configItems.defaultAddress - }, - to: { - name: [].concat(subscription.firstName || []).concat(subscription.lastName || []).join(' '), - address: email - }, - subject: util.format(subject, list.name), - encryptionKeys - }, { - html, - text, - data - }, err => { - if (err) { - log.error('Subscription', err); - } - }); - } - - let text = { - template: 'subscription/mail-' + template + '-text.hbs' - }; - - let html = { - template: 'subscription/mail-' + template + '-html.mjml.hbs', - layout: 'subscription/layout.mjml.hbs', - type: 'mjml' - }; - - helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { + function sendMail(html, text) { + mailer.sendMail({ + from: { + name: configItems.defaultFrom, + address: configItems.defaultAddress + }, + to: { + name: [].concat(subscription.firstName || []).concat(subscription.lastName || []).join(' '), + address: email + }, + subject: util.format(subject, list.name), + encryptionKeys + }, { + html, + text, + data + }, err => { if (err) { - return sendMail(html, text); + log.error('Subscription', err); } - - sendMail(tmpl.html, tmpl.text); }); + } - return callback(null, cid); + let text = { + template: 'subscription/mail-' + template + '-text.hbs' + }; + + let html = { + template: 'subscription/mail-' + template + '-html.mjml.hbs', + layout: 'subscription/layout.mjml.hbs', + type: 'mjml' + }; + + helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { + if (err) { + return sendMail(html, text); + } + + sendMail(tmpl.html, tmpl.text); }); + + return callback(); }); }); }); diff --git a/lib/tools.js b/lib/tools.js index 49d17e44..901ad0df 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -181,7 +181,7 @@ function validateEmail(address, checkBlocked, callback) { function getMessageLinks(serviceUrl, campaign, list, subscription) { return { - LINK_UNSUBSCRIBE: urllib.resolve(serviceUrl, '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid + '?auto=yes&c=' + campaign.cid), + LINK_UNSUBSCRIBE: urllib.resolve(serviceUrl, '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid + '?c=' + campaign.cid), LINK_PREFERENCES: urllib.resolve(serviceUrl, '/subscription/' + list.cid + '/manage/' + subscription.cid), LINK_BROWSER: urllib.resolve(serviceUrl, '/archive/' + campaign.cid + '/' + list.cid + '/' + subscription.cid), CAMPAIGN_ID: campaign.cid, diff --git a/routes/lists.js b/routes/lists.js index 4618c683..402c9095 100644 --- a/routes/lists.js +++ b/routes/lists.js @@ -784,12 +784,24 @@ function getUnsubscriptionModeOptions(unsubscriptionMode) { label: _('One-step (i.e. no email with confirmation link)') }; + options[lists.UnsubscriptionMode.ONE_STEP_WITH_FORM] = { + value: lists.UnsubscriptionMode.ONE_STEP_WITH_FORM, + selected: unsubscriptionMode === lists.UnsubscriptionMode.ONE_STEP_WITH_FORM, + label: _('One-step with unsubscription form (i.e. no email with confirmation link)') + }; + options[lists.UnsubscriptionMode.TWO_STEP] = { value: lists.UnsubscriptionMode.TWO_STEP, selected: unsubscriptionMode === lists.UnsubscriptionMode.TWO_STEP, label: _('Two-step (i.e. an email with confirmation link will be sent)') }; + options[lists.UnsubscriptionMode.TWO_STEP_WITH_FORM] = { + value: lists.UnsubscriptionMode.TWO_STEP_WITH_FORM, + selected: unsubscriptionMode === lists.UnsubscriptionMode.TWO_STEP_WITH_FORM, + label: _('Two-step with unsubscription form (i.e. an email with confirmation link will be sent)') + }; + options[lists.UnsubscriptionMode.MANUAL] = { value: lists.UnsubscriptionMode.MANUAL, selected: unsubscriptionMode === lists.UnsubscriptionMode.MANUAL, diff --git a/routes/subscription.js b/routes/subscription.js index e8f2c5f4..193ff63f 100644 --- a/routes/subscription.js +++ b/routes/subscription.js @@ -65,80 +65,23 @@ router.get('/confirm/:cid', (req, res, next) => { return next(err); } - settings.list(['defaultHomepage', 'serviceUrl', 'pgpPrivateKey', 'defaultAddress', 'defaultPostaddress', 'defaultFrom', 'disableConfirmations'], (err, configItems) => { - if (err) { - return next(err); - } + // FIXME - differentiate email based on action - // FIXME - split decision based on action + const relativeUrls = { + preferencesUrl: '/subscription/' + list.cid + '/manage/' + subscription.cid, + unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid + }; + + subscriptions.sendMail(list, subscription.email, 'subscription-confirmed', _('%s: Subscription Confirmed'), relativeUrls, {}, subscription, (err) => { + if (err) { + req.flash('danger', err.message || err); + log.error('Subscription', err); + return res.redirect('/subscription/' + encodeURIComponent(req.params.lcid) + '/unsubscribe/' + encodeURIComponent(req.body.cid) + '?' + tools.queryParams(req.body)); + } res.redirect('/subscription/' + list.cid + '/subscribed-notice'); - - if (configItems.disableConfirmations) { - return; - } - - fields.list(list.id, (err, fieldList) => { - if (err) { - return log.error('Fields', err); - } - - let encryptionKeys = []; - fields.getRow(fieldList, subscription).forEach(field => { - if (field.type === 'gpg' && field.value) { - encryptionKeys.push(field.value.trim()); - } - }); - - let sendMail = (html, text) => { - mailer.sendMail({ - from: { - name: configItems.defaultFrom, - address: configItems.defaultAddress - }, - to: { - name: [].concat(subscription.firstName || []).concat(subscription.lastName || []).join(' '), - address: subscription.email - }, - subject: util.format(_('%s: Subscription Confirmed'), list.name), - encryptionKeys - }, { - html, - text, - data: { - title: list.name, - homepage: configItems.defaultHomepage || configItems.serviceUrl, - contactAddress: configItems.defaultAddress, - defaultPostaddress: configItems.defaultPostaddress, - preferencesUrl: urllib.resolve(configItems.serviceUrl, '/subscription/' + list.cid + '/manage/' + subscription.cid), - unsubscribeUrl: urllib.resolve(configItems.serviceUrl, '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid) - } - }, err => { - if (err) { - log.error('Subscription', err); - } - }); - }; - - let text = { - template: 'subscription/mail-subscription-confirmed-text.hbs' - }; - - let html = { - template: 'subscription/mail-subscription-confirmed-html.mjml.hbs', - layout: 'subscription/layout.mjml.hbs', - type: 'mjml' - }; - - helpers.injectCustomFormTemplates(req.query.fid || list.defaultForm, { text, html }, (err, tmpl) => { - if (err) { - return sendMail(html, text); - } - - sendMail(tmpl.html, tmpl.text); - }); - }); }); + }); }); }); @@ -159,6 +102,8 @@ router.get('/:cid', passport.csrfProtection, (req, res, next) => { return next(err); } + // FIXME: process subscriber cid param for resubscription requests + let data = tools.convertKeys(req.query, { skip: ['layout'] }); @@ -561,6 +506,8 @@ router.post('/:lcid/manage-address', passport.parseForm, passport.csrfProtection }); router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next) => { + // FIXME: handle different subscription options. The one below is currently "One-step with unsubscribe form" + lists.getByCid(req.params.lcid, (err, list) => { if (!err && !list) { err = new Error(_('Selected list not found')); @@ -587,9 +534,9 @@ router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next) } subscription.lcid = req.params.lcid; + subscription.ucid = req.params.ucid; subscription.title = list.name; subscription.csrfToken = req.csrfToken(); - subscription.autosubmit = !!req.query.auto; subscription.campaign = req.query.c; subscription.defaultAddress = configItems.defaultAddress; subscription.defaultPostaddress = configItems.defaultPostaddress; @@ -636,83 +583,24 @@ router.post('/:lcid/unsubscribe', passport.parseForm, passport.csrfProtection, ( return next(err); } - let email = req.body.email; - - subscriptions.unsubscribe(list.id, email, req.body.campaign, (err, subscription) => { + subscriptions.unsubscribe(list.id, req.body.ucid, req.body.campaign, (err, subscription) => { if (err) { req.flash('danger', err.message || err); log.error('Subscription', err); - return res.redirect('/subscription/' + encodeURIComponent(req.params.lcid) + '/unsubscribe/' + encodeURIComponent(req.body.cid) + '?' + tools.queryParams(req.body)); + return res.redirect('/subscription/' + encodeURIComponent(req.params.lcid) + '/unsubscribe/' + encodeURIComponent(req.body.ucid) + '?' + tools.queryParams(req.body)); } - res.redirect('/subscription/' + req.params.lcid + '/unsubscribed-notice'); - fields.list(list.id, (err, fieldList) => { + const relativeUrls = { + subscribeUrl: '/subscription/' + list.cid + '?cid=' + subscription.cid + }; + subscriptions.sendMail(list, subscription.email, 'unsubscription-confirmed', _('%s: Unsubscribe Confirmed'), relativeUrls, {}, subscription, (err) => { if (err) { - return log.error('Fields', err); + req.flash('danger', err.message || err); + log.error('Subscription', err); + return res.redirect('/subscription/' + encodeURIComponent(list.cid) + '/unsubscribe/' + encodeURIComponent(subscription.cid) + '?' + tools.queryParams(req.body)); } - let encryptionKeys = []; - fields.getRow(fieldList, subscription).forEach(field => { - if (field.type === 'gpg' && field.value) { - encryptionKeys.push(field.value.trim()); - } - }); - - settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl', 'disableConfirmations'], (err, configItems) => { - if (err) { - return log.error('Settings', err); - } - - if (configItems.disableConfirmations) { - return; - } - - let sendMail = (html, text) => { - mailer.sendMail({ - from: { - name: configItems.defaultFrom, - address: configItems.defaultAddress - }, - to: { - name: [].concat(subscription.firstName || []).concat(subscription.lastName || []).join(' '), - address: subscription.email - }, - subject: util.format(_('%s: Unsubscribe Confirmed'), list.name), - encryptionKeys - }, { - html, - text, - data: { - title: list.name, - contactAddress: configItems.defaultAddress, - defaultPostaddress: configItems.defaultPostaddress, - subscribeUrl: urllib.resolve(configItems.serviceUrl, '/subscription/' + list.cid + '?cid=' + subscription.cid) - } - }, err => { - if (err) { - log.error('Subscription', err); - } - }); - }; - - let text = { - template: 'subscription/mail-unsubscription-confirmed-text.hbs' - }; - - let html = { - template: 'subscription/mail-unsubscription-confirmed-html.mjml.hbs', - layout: 'subscription/layout.mjml.hbs', - type: 'mjml' - }; - - helpers.injectCustomFormTemplates(req.query.fid || list.defaultForm, { text, html }, (err, tmpl) => { - if (err) { - return sendMail(html, text); - } - - sendMail(tmpl.html, tmpl.text); - }); - }); + res.redirect('/subscription/' + req.params.lcid + '/unsubscribed-notice'); }); }); }); diff --git a/services/sender.js b/services/sender.js index 0610a7bb..ea3e05de 100644 --- a/services/sender.js +++ b/services/sender.js @@ -418,7 +418,7 @@ function formatMessage(message, callback) { } }, list: { - unsubscribe: url.resolve(configItems.serviceUrl, '/subscription/' + list.cid + '/unsubscribe/' + message.subscription.cid + '?auto=yes') + unsubscribe: url.resolve(configItems.serviceUrl, '/subscription/' + list.cid + '/unsubscribe/' + message.subscription.cid) }, subject: tools.formatMessage(configItems.serviceUrl, campaign, list, message.subscription, campaign.subject), html: renderedHtml, diff --git a/views/subscription/partials/subscription-unsubscribe-form.hbs b/views/subscription/partials/subscription-unsubscribe-form.hbs index 8a11ebe2..4f40dcdc 100644 --- a/views/subscription/partials/subscription-unsubscribe-form.hbs +++ b/views/subscription/partials/subscription-unsubscribe-form.hbs @@ -1,20 +1,13 @@
- +
- +
-{{#if email}} - {{#if autosubmit}} - - {{/if}} -{{/if}} diff --git a/views/subscription/web-unsubscribe.mjml.hbs b/views/subscription/web-unsubscribe.mjml.hbs index a34892d4..4ea3c4fa 100644 --- a/views/subscription/web-unsubscribe.mjml.hbs +++ b/views/subscription/web-unsubscribe.mjml.hbs @@ -3,9 +3,6 @@ {{#translate}}Unsubscribe{{/translate}} - - {{#translate}}Enter your email address to unsubscribe from:{{/translate}} {{title}} - {{> subscription_unsubscribe_form}}