Merge pull request #707 from calzoneman/nodemailer-upgrade

Upgrade nodemailer to 4.x

NEWS.md

@@ -1,3 +1,20 @@
+2017-09-26
+==========
+
+**Breaking change:** the `nodemailer` dependency has been upgraded to version
+4.x.  I also took this opportunity to make some modifications to the email
+configuration and move it out of `config.yaml` to `conf/email.toml`.
+
+To upgrade:
+
+  * Run `npm upgrade` (or `rm -rf node_modules; npm install`)
+  * Copy `conf/example/email.toml` to `conf/email.toml`
+  * Edit `conf/email.toml` to your liking
+  * Remove the `mail:` block from `config.yaml`
+
+This feature only supports sending via SMTP for now.  If there is demand for
+other transports, feel free to open an issue or submit a pull request.
+
 2017-09-19
 ==========
 

conf/example/email.toml

@@ -0,0 +1,41 @@
+# SMTP configuration for sending mail
+[smtp]
+host = 'smtp.gmail.com'
+port = 465
+secure = true
+user = 'some-user@example.com'
+password = 'secretpassword'
+
+# Email configuration for password reset emails
+# Be sure to update both html-template AND text-template
+# nodemailer will send both and the email client will render whichever one is supported
+[password-reset]
+enabled = true
+
+# Template to use for HTML-formatted emails
+# $user$ will be replaced by the username for which the reset was requested
+# $url$ will be replaced by the password reset confirmation link
+html-template = """
+Hi $user$,<br>
+<br>
+A password reset was requested for your account on CHANGE ME.  You can complete the reset by opening the following link in your browser: <a href="$url$">$url$</a><br>
+<br>
+This link will expire in 24 hours.<br>
+<br>
+This email address is not monitored for replies.  For assistance with password resets, please <a href="http://example.com/contact">contact an administrator</a>.
+"""
+
+# Template to use for plaintext emails
+# Same substitutions as the HTML template
+text-template = """
+Hi $user$,
+
+A password reset was requested for your account on CHANGE ME.  You can complete the reset by opening the following link in your browser: $url$
+
+This link will expire in 24 hours.
+
+This email address is not monitored for replies.  For assistance with password resets, please contact an administrator.  See http://example.com/contact for contact information.
+"""
+
+from = "Example Website <website@example.com>"
+subject = "Password reset request"

config.template.yaml

@@ -114,18 +114,6 @@ io:
   # more CPU and will bottleneck pretty quickly under heavy load.
   per-message-deflate: false
 
-# Mailer details (used for sending password reset links)
-# see https://github.com/andris9/Nodemailer
-mail:
-  enabled: false
-  config:
-    service: 'Gmail'
-    auth:
-      user: 'some.user@gmail.com'
-      pass: 'supersecretpassword'
-  from-address: 'some.user@gmail.com'
-  from-name: 'CyTube Services'
-
 # YouTube v3 API key
 # See https://developers.google.com/youtube/registering_an_application
 # YouTube links will not work without this!

package.json

@@ -2,7 +2,7 @@
   "author": "Calvin Montgomery",
   "name": "CyTube",
   "description": "Online media synchronizer and chat",
-  "version": "3.49.0",
+  "version": "3.50.0",
   "repository": {
     "url": "http://github.com/calzoneman/sync"
   },
@@ -30,7 +30,7 @@
     "lodash": "^4.13.1",
     "morgan": "^1.6.1",
     "mysql": "^2.9.0",
-    "nodemailer": "^1.4.0",
+    "nodemailer": "^4.1.1",
     "prom-client": "^10.0.2",
     "proxy-addr": "^1.1.4",
     "pug": "^2.0.0-beta3",

src/config.js

@@ -1,12 +1,12 @@
 var fs = require("fs");
 var path = require("path");
-var nodemailer = require("nodemailer");
 var net = require("net");
 var YAML = require("yamljs");
 
 import { loadFromToml } from './configuration/configloader';
 import { CamoConfig } from './configuration/camoconfig';
 import { PrometheusConfig } from './configuration/prometheusconfig';
+import { EmailConfig } from './configuration/emailconfig';
 
 const LOGGER = require('@calzoneman/jsli')('config');
 
@@ -64,13 +64,6 @@ var defaults = {
         "ip-connection-limit": 10,
         "per-message-deflate": false
     },
-    mail: {
-        enabled: false,
-        /* the key "config" is omitted because the format depends on the
-           service the owner is configuring for nodemailer */
-        "from-address": "some.user@gmail.com",
-        "from-name": "CyTube Services"
-    },
     "youtube-v3-key": "",
     "channel-blacklist": [],
     "channel-path": "r",
@@ -141,6 +134,7 @@ function merge(obj, def, path) {
 var cfg = defaults;
 let camoConfig = new CamoConfig();
 let prometheusConfig = new PrometheusConfig();
+let emailConfig = new EmailConfig();
 
 /**
  * Initializes the configuration from the given YAML file
@@ -171,61 +165,82 @@ exports.load = function (file) {
         return;
     }
 
-    var mailconfig = {};
+    if (cfg.mail) {
-    if (cfg.mail && cfg.mail.config) {
+        LOGGER.error(
-        mailconfig = cfg.mail.config;
+            'Old style mail configuration found in config.yaml.  ' +
-        delete cfg.mail.config;
+            'Email will not be delivered unless you copy conf/example/email.toml ' +
+            'to conf/email.toml and edit it to your liking.  ' +
+            'To remove this warning, delete the "mail:" block in config.yaml.'
+        );
     }
+
     merge(cfg, defaults, "config");
-    cfg.mail.config = mailconfig;
 
     preprocessConfig(cfg);
     LOGGER.info("Loaded configuration from " + file);
 
     loadCamoConfig();
     loadPrometheusConfig();
+    loadEmailConfig();
 };
 
-function loadCamoConfig() {
+function checkLoadConfig(configClass, filename) {
     try {
-        camoConfig = loadFromToml(CamoConfig,
+        return loadFromToml(
-                                  path.resolve(__dirname, '..', 'conf', 'camo.toml'));
+            configClass,
-        const enabled = camoConfig.isEnabled() ? 'ENABLED' : 'DISABLED';
+            path.resolve(__dirname, '..', 'conf', filename)
-        LOGGER.info(`Loaded camo configuration from conf/camo.toml.  Camo is ${enabled}`);
+        );
     } catch (error) {
         if (error.code === 'ENOENT') {
-            LOGGER.info('No camo configuration found, chat images will not be proxied.');
+            return null;
-            camoConfig = new CamoConfig();
-            return;
         }
 
         if (typeof error.line !== 'undefined') {
-            LOGGER.error(`Error in conf/camo.toml: ${error} (line ${error.line})`);
+            LOGGER.error(`Error in conf/${fileanme}: ${error} (line ${error.line})`);
         } else {
-            LOGGER.error(`Error loading conf/camo.toml: ${error.stack}`);
+            LOGGER.error(`Error loading conf/${filename}: ${error.stack}`);
         }
     }
 }
 
-function loadPrometheusConfig() {
+function loadCamoConfig() {
-    try {
+    const conf = checkLoadConfig(CamoConfig, 'camo.toml');
-        prometheusConfig = loadFromToml(PrometheusConfig,
-                path.resolve(__dirname, '..', 'conf', 'prometheus.toml'));
-        const enabled = prometheusConfig.isEnabled() ? 'ENABLED' : 'DISABLED';
-        LOGGER.info('Loaded prometheus configuration from conf/prometheus.toml.  '
-                + `Prometheus listener is ${enabled}`);
-    } catch (error) {
-        if (error.code === 'ENOENT') {
-            LOGGER.info('No prometheus configuration found, defaulting to disabled');
-            prometheusConfig = new PrometheusConfig();
-            return;
-        }
 
-        if (typeof error.line !== 'undefined') {
+    if (conf === null) {
-            LOGGER.error(`Error in conf/prometheus.toml: ${error} (line ${error.line})`);
+        LOGGER.info('No camo configuration found, chat images will not be proxied.');
-        } else {
+        camoConfig = new CamoConfig();
-            LOGGER.error(`Error loading conf/prometheus.toml: ${error.stack}`);
+    } else {
-        }
+        camoConfig = conf;
+        const enabled = camoConfig.isEnabled() ? 'ENABLED' : 'DISABLED';
+        LOGGER.info(`Loaded camo configuration from conf/camo.toml.  Camo is ${enabled}`);
+    }
+}
+
+function loadPrometheusConfig() {
+    const conf = checkLoadConfig(PrometheusConfig, 'prometheus.toml');
+
+    if (conf === null) {
+        LOGGER.info('No prometheus configuration found, defaulting to disabled');
+        prometheusConfig = new PrometheusConfig();
+    } else {
+        prometheusConfig = conf;
+        const enabled = prometheusConfig.isEnabled() ? 'ENABLED' : 'DISABLED';
+        LOGGER.info(
+            'Loaded prometheus configuration from conf/prometheus.toml.  ' +
+            `Prometheus listener is ${enabled}`
+        );
+    }
+}
+
+function loadEmailConfig() {
+    const conf = checkLoadConfig(EmailConfig, 'email.toml');
+
+    if (conf === null) {
+        LOGGER.info('No email configuration found, defaulting to disabled');
+        emailConfig = new EmailConfig();
+    } else {
+        emailConfig = conf;
+        LOGGER.info('Loaded email configuration from conf/email.toml.');
     }
 }
 
@@ -240,11 +255,6 @@ function preprocessConfig(cfg) {
     }
     cfg.http["root-domain-dotted"] = root;
 
-    // Setup nodemailer
-    cfg.mail.nodemailer = nodemailer.createTransport(
-        cfg.mail.config
-    );
-
     // Debug
     if (process.env.DEBUG === "1" || process.env.DEBUG === "true") {
         cfg.debug = true;
@@ -450,3 +460,7 @@ exports.getCamoConfig = function getCamoConfig() {
 exports.getPrometheusConfig = function getPrometheusConfig() {
     return prometheusConfig;
 };
+
+exports.getEmailConfig = function getEmailConfig() {
+    return emailConfig;
+};

src/configuration/emailconfig.js

@@ -0,0 +1,61 @@
+class EmailConfig {
+    constructor(config = { 'password-reset': { enabled: false }, smtp: {} }) {
+        this.config = config;
+
+        const smtp = config.smtp;
+        this._smtp = {
+            getHost() {
+                return smtp.host;
+            },
+
+            getPort() {
+                return smtp.port;
+            },
+
+            isSecure() {
+                return smtp.secure;
+            },
+
+            getUser() {
+                return smtp.user;
+            },
+
+            getPassword() {
+                return smtp.password;
+            }
+        }
+
+        const reset = config['password-reset'];
+        this._reset = {
+            isEnabled() {
+                return reset.enabled;
+            },
+
+            getHTML() {
+                return reset['html-template'];
+            },
+
+            getText() {
+                return reset['text-template'];
+            },
+
+            getFrom() {
+                return reset.from;
+            },
+
+            getSubject() {
+                return reset.subject;
+            }
+        };
+    }
+
+    getSmtp() {
+        return this._smtp;
+    }
+
+    getPasswordReset() {
+        return this._reset;
+    }
+}
+
+export { EmailConfig };

src/controller/email.js

@@ -0,0 +1,31 @@
+class EmailController {
+    constructor(mailer, config) {
+        this.mailer = mailer;
+        this.config = config;
+    }
+
+    async sendPasswordReset(params = {}) {
+        const { address, username, url } = params;
+
+        const resetConfig = this.config.getPasswordReset();
+
+        const html = resetConfig.getHTML()
+                .replace(/\$user\$/g, username)
+                .replace(/\$url\$/g, url);
+        const text = resetConfig.getText()
+                .replace(/\$user\$/g, username)
+                .replace(/\$url\$/g, url);
+
+        const result = await this.mailer.sendMail({
+            from: resetConfig.getFrom(),
+            to: `${username} <${address}>`,
+            subject: resetConfig.getSubject(),
+            html,
+            text
+        });
+
+        return result;
+    }
+}
+
+export { EmailController };

src/server.js

@@ -55,6 +55,7 @@ import { Gauge } from 'prom-client';
 import { AccountDB } from './db/account';
 import { ChannelDB } from './db/channel';
 import { AccountController } from './controller/account';
+import { EmailController } from './controller/email';
 
 var Server = function () {
     var self = this;
@@ -89,8 +90,34 @@ var Server = function () {
     const accountDB = new AccountDB(db.getDB());
     const channelDB = new ChannelDB(db.getDB());
 
+    // controllers
     const accountController = new AccountController(accountDB, globalMessageBus);
 
+    let emailTransport;
+    if (Config.getEmailConfig().getPasswordReset().isEnabled()) {
+        const smtpConfig = Config.getEmailConfig().getSmtp();
+        emailTransport = require("nodemailer").createTransport({
+            host: smtpConfig.getHost(),
+            port: smtpConfig.getPort(),
+            secure: smtpConfig.isSecure(),
+            auth: {
+                user: smtpConfig.getUser(),
+                pass: smtpConfig.getPassword()
+            }
+        });
+    } else {
+        emailTransport = {
+            sendMail() {
+                throw new Error('Email is not enabled on this server')
+            }
+        };
+    }
+
+    const emailController = new EmailController(
+        emailTransport,
+        Config.getEmailConfig()
+    );
+
     // webserver init -----------------------------------------------------
     const ioConfig = IOConfiguration.fromOldConfig(Config);
     const webConfig = WebConfiguration.fromOldConfig(Config);
@@ -104,7 +131,8 @@ var Server = function () {
         channelIndex = new LocalChannelIndex();
     }
     self.express = express();
-    require("./web/webserver").init(self.express,
+    require("./web/webserver").init(
+            self.express,
             webConfig,
             ioConfig,
             clusterClient,
@@ -112,7 +140,10 @@ var Server = function () {
             session,
             globalMessageBus,
             accountController,
-            channelDB);
+            channelDB,
+            Config.getEmailConfig(),
+            emailController
+    );
 
     // http/https/sio server init -----------------------------------------
     var key = "", cert = "", ca = undefined;

src/web/account.js

@@ -18,6 +18,8 @@ const url = require("url");
 const LOGGER = require('@calzoneman/jsli')('database/accounts');
 
 let globalMessageBus;
+let emailConfig;
+let emailController;
 
 /**
  * Handles a GET request for /account/edit
@@ -531,7 +533,7 @@ function handlePasswordReset(req, res) {
             Logger.eventlog.log("[account] " + ip + " requested password recovery for " +
                                 name + " <" + email + ">");
 
-            if (!Config.get("mail.enabled")) {
+            if (!emailConfig.getPasswordReset().isEnabled()) {
                 sendPug(res, "account-passwordreset", {
                     reset: false,
                     resetEmail: email,
@@ -541,37 +543,26 @@ function handlePasswordReset(req, res) {
                 return;
             }
 
-            var msg = "A password reset request was issued for your " +
+            const baseUrl = `${req.realProtocol}://${req.header("host")}`;
-                      "account `"+ name + "` on " + Config.get("http.domain") +
-                      ".  This request is valid for 24 hours.  If you did "+
-                      "not initiate this, there is no need to take action."+
-                      "  To reset your password, copy and paste the " +
-                      "following link into your browser: " +
-                      Config.get("http.domain") + "/account/passwordrecover/"+hash;
 
-            var mail = {
+            emailController.sendPasswordReset({
-                from: Config.get("mail.from-name") + " <" + Config.get("mail.from-address") + ">",
+                username: name,
-                to: email,
+                address: email,
-                subject: "Password reset request",
+                url: `${baseUrl}/account/passwordrecover/${hash}`
-                text: msg
+            }).then(result => {
-            };
+                sendPug(res, "account-passwordreset", {
-
+                    reset: true,
-            Config.get("mail.nodemailer").sendMail(mail, function (err, response) {
+                    resetEmail: email,
-                if (err) {
+                    resetErr: false
-                    LOGGER.error("mail fail: " + err);
+                });
-                    sendPug(res, "account-passwordreset", {
+            }).catch(error => {
-                        reset: false,
+                LOGGER.error("Sending password reset email failed: %s", error);
-                        resetEmail: email,
+                sendPug(res, "account-passwordreset", {
-                        resetErr: "Sending reset email failed.  Please contact an " +
+                    reset: false,
-                                  "administrator for assistance."
+                    resetEmail: email,
-                    });
+                    resetErr: "Sending reset email failed.  Please contact an " +
-                } else {
+                              "administrator for assistance."
-                    sendPug(res, "account-passwordreset", {
+                });
-                        reset: true,
-                        resetEmail: email,
-                        resetErr: false
-                    });
-                }
             });
         });
     });
@@ -639,8 +630,10 @@ module.exports = {
     /**
      * Initialize the module
      */
-    init: function (app, _globalMessageBus) {
+    init: function (app, _globalMessageBus, _emailConfig, _emailController) {
         globalMessageBus = _globalMessageBus;
+        emailConfig = _emailConfig;
+        emailController = _emailController;
 
         app.get("/account/edit", handleAccountEditPage);
         app.post("/account/edit", handleAccountEdit);

src/web/webserver.js

@@ -171,7 +171,9 @@ module.exports = {
         session,
         globalMessageBus,
         accountController,
-        channelDB
+        channelDB,
+        emailConfig,
+        emailController
     ) {
         patchExpressToHandleAsync();
         const chanPath = Config.get('channel-path');
@@ -230,7 +232,7 @@ module.exports = {
         require('./routes/socketconfig')(app, clusterClient);
         require('./routes/contact')(app, webConfig);
         require('./auth').init(app);
-        require('./account').init(app, globalMessageBus);
+        require('./account').init(app, globalMessageBus, emailConfig, emailController);
         require('./acp').init(app);
         require('../google2vtt').attach(app);
         require('./routes/google_drive_userscript')(app);

test/controller/email.js

@@ -0,0 +1,50 @@
+const assert = require('assert');
+const { createTransport } = require('nodemailer');
+const { EmailController } = require('../../lib/controller/email');
+const { EmailConfig } = require('../../lib/configuration/emailconfig');
+
+describe('EmailController', () => {
+    describe('sendPasswordReset', () => {
+        it('sends a password reset email', () => {
+            const mailer = createTransport({
+                jsonTransport: true
+            });
+            const config = new EmailConfig({
+                'password-reset': {
+                    from: 'Test <test@example.com>',
+                    subject: 'Password Reset',
+                    'html-template': 'Reset <a href="$url$">here</a> $user$',
+                    'text-template': 'Text is better than HTML $user$ $url$'
+                }
+            });
+
+            const controller = new EmailController(mailer, config);
+
+            return controller.sendPasswordReset({
+                address: 'some-user@example.com',
+                username: 'SomeUser',
+                url: 'http://localhost/password-reset/blah'
+            }).then(info => {
+                const sentMessage = JSON.parse(info.message);
+
+                assert.strictEqual(sentMessage.subject, 'Password Reset');
+                assert.deepStrictEqual(
+                    sentMessage.from,
+                    { name: 'Test', address: 'test@example.com' }
+                );
+                assert.deepStrictEqual(
+                    sentMessage.to,
+                    [{ name: 'SomeUser', address: 'some-user@example.com' }]
+                );
+                assert.strictEqual(
+                    sentMessage.html,
+                    'Reset <a href="http://localhost/password-reset/blah">here</a> SomeUser'
+                );
+                assert.strictEqual(
+                    sentMessage.text,
+                    'Text is better than HTML SomeUser http://localhost/password-reset/blah'
+                );
+            });
+        });
+    });
+});