From 7d9b24c5b9894a287786d6e9634436bc4703b72d Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 14 Nov 2016 10:44:33 -0700 Subject: [PATCH 01/15] start adding mail settings --- models/setting.js | 10 +++++++++- package.json | 2 ++ routes/api/settings/index.js | 6 +++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/models/setting.js b/models/setting.js index 09b13e099..fb5a9d062 100644 --- a/models/setting.js +++ b/models/setting.js @@ -1,9 +1,17 @@ const mongoose = require('../mongoose'); const Schema = mongoose.Schema; +/** + * this Schema manages application settings that get used on front- and backend + * NOTE: when you set a setting here, it will not automatically be exposed to + * the front end. You must add it to the whitelist in the settings route + * in /routes/api/settings/index.js + * @type {Schema} + */ const SettingSchema = new Schema({ id: {type: String, default: '1'}, - moderation: {type: String, enum: ['pre', 'post'], default: 'pre'} + moderation: {type: String, enum: ['pre', 'post'], default: 'pre'}, + smtp_connection_string: {type: String, default: ''}, }, { timestamps: { createdAt: 'created_at', diff --git a/package.json b/package.json index cbdaad174..db7117b47 100644 --- a/package.json +++ b/package.json @@ -51,8 +51,10 @@ "debug": "^2.2.0", "ejs": "^2.5.2", "express": "^4.14.0", + "lodash": "^4.16.6", "mongoose": "^4.6.5", "morgan": "^1.7.0", + "nodemailer": "^2.6.4", "prompt": "^1.0.0", "uuid": "^2.0.3" }, diff --git a/routes/api/settings/index.js b/routes/api/settings/index.js index 585bf083a..2665cacc8 100644 --- a/routes/api/settings/index.js +++ b/routes/api/settings/index.js @@ -1,3 +1,4 @@ +const _ = require('lodash'); const express = require('express'); const router = express.Router(); const Setting = require('../../../models/setting'); @@ -5,7 +6,10 @@ const Setting = require('../../../models/setting'); router.get('/', (req, res, next) => { Setting .getSettings() - .then(settings => res.json(settings)) + .then(settings => { + const whitelist = ['moderation']; + res.json(_.pick(settings, whitelist)); + }) .catch(next); }); From c1271d4d1bbaf470b3009cc3bd0eccc99e40a8a5 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Mon, 14 Nov 2016 13:31:47 -0700 Subject: [PATCH 02/15] mailer module v1 --- .editorconfig | 2 +- package.json | 1 + services/mailer.js | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 services/mailer.js diff --git a/.editorconfig b/.editorconfig index 9c378bd19..986314661 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,7 +1,7 @@ root = true [*] -indent_style = tab +indent_style = space end_of_line = lf charset = utf-8 trim_trailing_whitespace = true diff --git a/package.json b/package.json index db7117b47..110a7c47d 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ "mongoose": "^4.6.5", "morgan": "^1.7.0", "nodemailer": "^2.6.4", + "nodemailer-sendgrid-transport": "^0.2.0", "prompt": "^1.0.0", "uuid": "^2.0.3" }, diff --git a/services/mailer.js b/services/mailer.js new file mode 100644 index 000000000..ec1dd5f5e --- /dev/null +++ b/services/mailer.js @@ -0,0 +1,52 @@ +const nodemailer = require('nodemailer'); +const sgTransport = require('nodemailer-sendgrid-transport'); +const options = { + auth: { + api_key: process.env.TALK_SENDGRID_APIKEY + } +}; + +const transporter = nodemailer.createTransport(sgTransport(options)); + +transporter.sendMail({ + from: 'support@mrdavis.com', + to: 'riley.davis@gmail.com', + subject: 'this is only a test', + text: 'this is the body of the email maybe?', + html: ` + + + + + + + + +
foobar
rileydavis
` +}); + +const mailer = { + /** + * sendSimple + * + * @param {Object} {from, to, subject, text = '', html = ''} + * @returns + */ + sendSimple ({from, to, subject, text = '', html = ''}) { + return new Promise((resolve, reject) => { + if (!from) { + reject('sendSimple requires a from address'); + } + if (!to) { + reject('sendSimple requires a comma-separated list of "to" addresses'); + } + if (!subject) { + reject('sendSimple requires a subject for the email'); + } + + return resolve(transporter.sendMail({from, to, subject, text, html})); + }); + } +}; + +module.exports = mailer; From c86e1e8d47452a03470b0c35640b1355f1780756 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Tue, 15 Nov 2016 16:21:25 -0700 Subject: [PATCH 03/15] create and decode jwt --- models/user.js | 24 +++++++++++++++++++++++- package.json | 3 ++- routes/admin/index.js | 14 +++++++++++++- routes/api/user/index.js | 34 ++++++++++++++++++++++++++++++++++ services/mailer.js | 17 ----------------- 5 files changed, 72 insertions(+), 20 deletions(-) diff --git a/models/user.js b/models/user.js index c6197a220..f4edc8f63 100644 --- a/models/user.js +++ b/models/user.js @@ -1,6 +1,7 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); const bcrypt = require('bcrypt'); +const jwt = require('jsonwebtoken'); const SALT_ROUNDS = 10; @@ -83,10 +84,15 @@ UserSchema.options.toObject.transform = (doc, ret, options) => { * @param {Function} done [description] */ UserSchema.statics.findLocalUser = function(email, password) { + + if (!email || typeof email !== 'string') { + return Promise.reject('email is required for findLocalUser'); + } + return User.findOne({ profiles: { $elemMatch: { - id: email, + id: email.toLowerCase(), provider: 'local' } } @@ -221,6 +227,8 @@ UserSchema.statics.createLocalUser = function(email, password, displayName) { return Promise.reject('email is required'); } + email = email.toLowerCase(); + if (!password) { return Promise.reject('password is required'); } @@ -338,6 +346,20 @@ UserSchema.statics.findByIdArray = function(ids) { }); }; +UserSchema.statics.createJWT = function (email) { + if (!email || typeof email !== 'string') { + return Promise.reject('email is required when creating a JWT for resetting passord'); + } + + email = email.toLowerCase(); + + const payload = {email, jti: uuid.v4()}; + + const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); + + return Promise.resolve(token); +}; + const User = mongoose.model('User', UserSchema); module.exports = User; diff --git a/package.json b/package.json index ad0e2dbb0..76173b7aa 100644 --- a/package.json +++ b/package.json @@ -51,13 +51,13 @@ "debug": "^2.2.0", "ejs": "^2.5.2", "express": "^4.14.0", + "jsonwebtoken": "^7.1.9", "lodash": "^4.16.6", "mongoose": "^4.6.5", "morgan": "^1.7.0", "nodemailer": "^2.6.4", "nodemailer-sendgrid-transport": "^0.2.0", "prompt": "^1.0.0", - "react-mdl-selectfield": "^0.2.0", "uuid": "^2.0.3" }, "devDependencies": { @@ -107,6 +107,7 @@ "react": "15.3.2", "react-dom": "15.3.2", "react-mdl": "^1.7.2", + "react-mdl-selectfield": "^0.2.0", "react-onclickoutside": "^5.7.1", "react-redux": "^4.4.5", "react-router": "^3.0.0", diff --git a/routes/admin/index.js b/routes/admin/index.js index 52957f54a..84c130ba8 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,11 +1,23 @@ const express = require('express'); - +const jwt = require('jsonwebtoken'); const router = express.Router(); router.get('/embed/stream/preview', (req, res) => { res.render('embed-stream', {basePath: '/client/embed/stream'}); }); +router.get('/password-reset/:token', (req, res, next) => { + jwt.verify(req.params.token, process.env.TALK_SESSION_SECRET, (error, decoded) => { + if (error) { + return res.status(400).json({error}); + } + + console.log(decoded); + + res.json(decoded); + }); +}); + router.get('*', (req, res) => { res.render('admin', {basePath: '/client/coral-admin'}); }); diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 4665f4e52..c0bc43c6a 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -1,6 +1,7 @@ const express = require('express'); const router = express.Router(); const User = require('../../../models/user'); +const mailer = require('../../../services/mailer'); router.get('/', (req, res, next) => { const { @@ -70,4 +71,37 @@ router.post('/:user_id/role', (req, res, next) => { .catch(next); }); +/** + * this endpoint takes an email (username) and checks if it belongs to a User account + * if it does, create a JWT and send an email + */ +router.post('/request-password-reset', (req, res, next) => { + const {email} = req.body; + + console.log('/request-password-reset', req.body); + + if (!email) { + return next(); + } + + User + .createJWT(email) + .then(token => { + const options = { + subject: 'password reset requested', + from: 'coralcore@mozillafoundation.org', + to: 'riley.davis@gmail.com', + html: `reset password` + }; + return mailer.sendSimple(options); + }) + .then(success => { + console.log(success); + res.json({success: true}); + }) + .catch(error => { + res.status(500).json({error}); + }); +}); + module.exports = router; diff --git a/services/mailer.js b/services/mailer.js index ec1dd5f5e..eb900d782 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -8,23 +8,6 @@ const options = { const transporter = nodemailer.createTransport(sgTransport(options)); -transporter.sendMail({ - from: 'support@mrdavis.com', - to: 'riley.davis@gmail.com', - subject: 'this is only a test', - text: 'this is the body of the email maybe?', - html: ` - - - - - - - - -
foobar
rileydavis
` -}); - const mailer = { /** * sendSimple From 7df7279323c2fa017b7907421aef9a55dd6aa34a Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 16 Nov 2016 11:49:58 -0700 Subject: [PATCH 04/15] endpoints for updating password --- models/user.js | 42 +++++++++++++++++++++++++++++++++++++--- routes/admin/index.js | 12 ++---------- routes/api/user/index.js | 25 +++++++++++++++++++++++- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/models/user.js b/models/user.js index f4edc8f63..c3c81010a 100644 --- a/models/user.js +++ b/models/user.js @@ -196,6 +196,7 @@ UserSchema.statics.changePassword = function(id, password) { }) .then((hashedPassword) => { return User.update({id}, { + $inc: {__v: 1}, $set: { password: hashedPassword } @@ -346,6 +347,10 @@ UserSchema.statics.findByIdArray = function(ids) { }); }; +/** + * Creates a JWT from a user email. Only works for local accounts. + * @param {String} email of the local user + */ UserSchema.statics.createJWT = function (email) { if (!email || typeof email !== 'string') { return Promise.reject('email is required when creating a JWT for resetting passord'); @@ -353,11 +358,42 @@ UserSchema.statics.createJWT = function (email) { email = email.toLowerCase(); - const payload = {email, jti: uuid.v4()}; + return this.findOne({profiles: {$elemMatch: {id: email}}}) + .then(user => { - const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); + if (user === null) { + return Promise.reject(`Uh oh! We've never heard of ${email}. Maybe there's a typo in there?`); + } - return Promise.resolve(token); + const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; + const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); + + return token; + }); +}; + +/** + * verifies a jwt and returns the associated user + * @param {String} token the JSON Web Token to verify + */ +UserSchema.statics.verifyToken = function (token) { + return new Promise((resolve, reject) => { + jwt.verify(token, process.env.TALK_SESSION_SECRET, (error, decoded) => { + if (error) { + return reject(error); + } + + resolve(decoded); + }); + }) + .then(decoded => { + /** + * TODO: check the jti from this decoded token in redis + * and make an entry if it does not exist. + * reject if entry already exists. + */ + return this.findOne({id: decoded.userId}); + }); }; const User = mongoose.model('User', UserSchema); diff --git a/routes/admin/index.js b/routes/admin/index.js index 84c130ba8..2b967708a 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,5 +1,4 @@ const express = require('express'); -const jwt = require('jsonwebtoken'); const router = express.Router(); router.get('/embed/stream/preview', (req, res) => { @@ -7,15 +6,8 @@ router.get('/embed/stream/preview', (req, res) => { }); router.get('/password-reset/:token', (req, res, next) => { - jwt.verify(req.params.token, process.env.TALK_SESSION_SECRET, (error, decoded) => { - if (error) { - return res.status(400).json({error}); - } - - console.log(decoded); - - res.json(decoded); - }); + // render a page or something? + res.send('ok'); }); router.get('*', (req, res) => { diff --git a/routes/api/user/index.js b/routes/api/user/index.js index c0bc43c6a..8cbebfd1f 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -71,6 +71,27 @@ router.post('/:user_id/role', (req, res, next) => { .catch(next); }); +/** + * expects 2 fields in the body of the request + * 1) the token that was in the url of the email link {String} + * 2) the new password {String} + */ +router.post('/update-password', (req, res, next) => { + const {token, password} = req.body; + + User.verifyToken(token) + .then(user => { + return User.changePassword(user.id, password); + }) + .then(() => { + res.status(204).end(); + }) + .catch(error => { + console.error(error); + res.status(401).send('Not Authorized'); + }); +}); + /** * this endpoint takes an email (username) and checks if it belongs to a User account * if it does, create a JWT and send an email @@ -91,7 +112,9 @@ router.post('/request-password-reset', (req, res, next) => { subject: 'password reset requested', from: 'coralcore@mozillafoundation.org', to: 'riley.davis@gmail.com', - html: `reset password` + html: `

We received a request to reset your password. If you did not request this change, you can ignore this email. + If you did, please click here to reset password.

+ ${process.env.NODE_ENV === 'production' ? '' : `

${token}

`}` }; return mailer.sendSimple(options); }) From 1795fc8863c290da8ccac19bec97f8b8180c4397 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 16 Nov 2016 12:00:44 -0700 Subject: [PATCH 05/15] remove log --- routes/api/user/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 8cbebfd1f..67bc9197d 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -119,7 +119,6 @@ router.post('/request-password-reset', (req, res, next) => { return mailer.sendSimple(options); }) .then(success => { - console.log(success); res.json({success: true}); }) .catch(error => { From 9942d6eee40a80aa9305ffdac46a8aee3ffcbb91 Mon Sep 17 00:00:00 2001 From: riley Date: Wed, 16 Nov 2016 12:09:47 -0700 Subject: [PATCH 06/15] remove reference --- routes/api/user/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 67bc9197d..808ad6940 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -118,7 +118,7 @@ router.post('/request-password-reset', (req, res, next) => { }; return mailer.sendSimple(options); }) - .then(success => { + .then(() => { res.json({success: true}); }) .catch(error => { From 81299e8add5f4e15dcc194581196d956e26fa32c Mon Sep 17 00:00:00 2001 From: riley Date: Wed, 16 Nov 2016 13:20:07 -0700 Subject: [PATCH 07/15] better error reporting. log when env variable is unset --- models/user.js | 8 ++++++++ routes/api/user/index.js | 12 ++++++------ services/mailer.js | 8 ++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/models/user.js b/models/user.js index c3c81010a..7d083f70d 100644 --- a/models/user.js +++ b/models/user.js @@ -2,9 +2,17 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); +const debug = require('debug')('talk:user_model'); const SALT_ROUNDS = 10; +if (!process.env.TALK_SESSION_SECRET) { + debug('\n////////////////////////////////////////////////////////////\n' + + '/// TALK_SESSION_SECRET must be defined to encode ///\n' + + '/// JSON Web Tokens and other auth functionality ///\n' + + '////////////////////////////////////////////////////////////'); +} + const UserSchema = new mongoose.Schema({ id: { type: String, diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 808ad6940..c9b497659 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -99,8 +99,6 @@ router.post('/update-password', (req, res, next) => { router.post('/request-password-reset', (req, res, next) => { const {email} = req.body; - console.log('/request-password-reset', req.body); - if (!email) { return next(); } @@ -109,12 +107,12 @@ router.post('/request-password-reset', (req, res, next) => { .createJWT(email) .then(token => { const options = { - subject: 'password reset requested', + subject: 'Password Reset Requested - Talk', from: 'coralcore@mozillafoundation.org', - to: 'riley.davis@gmail.com', + to: email, html: `

We received a request to reset your password. If you did not request this change, you can ignore this email. If you did, please click here to reset password.

- ${process.env.NODE_ENV === 'production' ? '' : `

${token}

`}` + ${process.env.NODE_ENV === 'production' ? '' : `

${token}

`}` }; return mailer.sendSimple(options); }) @@ -122,7 +120,9 @@ router.post('/request-password-reset', (req, res, next) => { res.json({success: true}); }) .catch(error => { - res.status(500).json({error}); + const errorMsg = typeof error === 'string' ? error : error.message; + + res.status(500).json({error: errorMsg}); }); }); diff --git a/services/mailer.js b/services/mailer.js index eb900d782..89115d619 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,11 +1,19 @@ const nodemailer = require('nodemailer'); const sgTransport = require('nodemailer-sendgrid-transport'); +const debug = require('debug')('talk:mail'); const options = { auth: { api_key: process.env.TALK_SENDGRID_APIKEY } }; +if (!process.env.TALK_SENDGRID_APIKEY) { + debug('\n///////////////////////////////////////////////////////////////\n' + + '/// TALK_SENDGRID_APIKEY should be defined if you would ///\n' + + '/// like to send password reset emails from Talk ///\n' + + '///////////////////////////////////////////////////////////////'); +} + const transporter = nodemailer.createTransport(sgTransport(options)); const mailer = { From e027a162d6d46599e0cb30b04498ffc1831b7343 Mon Sep 17 00:00:00 2001 From: riley Date: Wed, 16 Nov 2016 14:00:33 -0700 Subject: [PATCH 08/15] remove sendgrid shim and use connection string --- package.json | 1 - services/mailer.js | 8 +------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/package.json b/package.json index 17d5bdc9f..b279050f6 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,6 @@ "mongoose": "^4.6.5", "morgan": "^1.7.0", "nodemailer": "^2.6.4", - "nodemailer-sendgrid-transport": "^0.2.0", "prompt": "^1.0.0", "uuid": "^2.0.3" }, diff --git a/services/mailer.js b/services/mailer.js index 89115d619..cf7a4d26f 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,11 +1,5 @@ const nodemailer = require('nodemailer'); -const sgTransport = require('nodemailer-sendgrid-transport'); const debug = require('debug')('talk:mail'); -const options = { - auth: { - api_key: process.env.TALK_SENDGRID_APIKEY - } -}; if (!process.env.TALK_SENDGRID_APIKEY) { debug('\n///////////////////////////////////////////////////////////////\n' + @@ -14,7 +8,7 @@ if (!process.env.TALK_SENDGRID_APIKEY) { '///////////////////////////////////////////////////////////////'); } -const transporter = nodemailer.createTransport(sgTransport(options)); +const transporter = nodemailer.createTransport(`smtps://apikey:${process.env.TALK_SENDGRID_APIKEY}@smtp.sendgrid.net`); const mailer = { /** From 967472ea74443d14dd69a4216c1c86b7de37f7b8 Mon Sep 17 00:00:00 2001 From: riley Date: Wed, 16 Nov 2016 14:38:29 -0700 Subject: [PATCH 09/15] udpate smtp module to take a full smtp connection string --- services/mailer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/mailer.js b/services/mailer.js index cf7a4d26f..d90d04071 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,14 +1,14 @@ const nodemailer = require('nodemailer'); const debug = require('debug')('talk:mail'); -if (!process.env.TALK_SENDGRID_APIKEY) { +if (!process.env.TALK_SMTP_STRING) { debug('\n///////////////////////////////////////////////////////////////\n' + '/// TALK_SENDGRID_APIKEY should be defined if you would ///\n' + '/// like to send password reset emails from Talk ///\n' + '///////////////////////////////////////////////////////////////'); } -const transporter = nodemailer.createTransport(`smtps://apikey:${process.env.TALK_SENDGRID_APIKEY}@smtp.sendgrid.net`); +const transporter = nodemailer.createTransport(process.env.TALK_SMTP_STRING); const mailer = { /** From 6e932b840ef11689240289a35312b98441884b20 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 16 Nov 2016 15:32:44 -0700 Subject: [PATCH 10/15] merge master. load reset password template from a view --- .../coral-plugin-commentbox/translations.json | 2 +- models/user.js | 159 +++++++++++++----- routes/api/user/index.js | 47 ++---- views/password-reset-email.ejs | 6 + 4 files changed, 146 insertions(+), 68 deletions(-) create mode 100644 views/password-reset-email.ejs diff --git a/client/coral-plugin-commentbox/translations.json b/client/coral-plugin-commentbox/translations.json index e8eb8ad52..29f3212b9 100644 --- a/client/coral-plugin-commentbox/translations.json +++ b/client/coral-plugin-commentbox/translations.json @@ -5,7 +5,7 @@ "comment": "Comment", "name": "Name", "comment-post-notif": "Your comment has been posted.", - "comment-post-notif-premod": "Thank you for reporting this comment. Our moderation team has been notified and will review it shortly." + "comment-post-notif-premod": "Thank you for posting. Our moderation team will review your comment shortly." }, "es": { "post": "Publicar", diff --git a/models/user.js b/models/user.js index 7d083f70d..bbf8f21e6 100644 --- a/models/user.js +++ b/models/user.js @@ -4,8 +4,16 @@ const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const debug = require('debug')('talk:user_model'); +// SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run +// through during the salting process. const SALT_ROUNDS = 10; +// USER_ROLES is the array of roles that is permissible as a user role. +const USER_ROLES = [ + 'admin', + 'moderator' +]; + if (!process.env.TALK_SESSION_SECRET) { debug('\n////////////////////////////////////////////////////////////\n' + '/// TALK_SESSION_SECRET must be defined to encode ///\n' + @@ -13,30 +21,62 @@ if (!process.env.TALK_SESSION_SECRET) { '////////////////////////////////////////////////////////////'); } +// UserSchema is the mongoose schema defined as the representation of a User in +// MongoDB. const UserSchema = new mongoose.Schema({ + + // This ID represents the most unique identifier for a user, it is generated + // when the user is created as a random uuid. id: { type: String, default: uuid.v4, unique: true, required: true }, + + // This is sourced from the social provider or set manually during user setup + // and simply provides a name to display for the given user. displayName: String, + + // This is true when the user account is disabled, no action should be + // acknowledged when they are disabled. Logins are also prevented. disabled: Boolean, + + // This provides a source of identity proof for users who login using the + // local provider. A local provider will be assumed for users who do not + // have any social profiles. password: String, - profiles: [{ + + // Profiles describes the array of identities for a given user. Any one user + // can have multiple profiles associated with them, including multiple email + // addresses. + profiles: [new mongoose.Schema({ + + // ID provides the identifier for the user profile, in the case of a local + // provider, the id would be an email, in the case of a social provider, + // the id would be the foreign providers identifier. id: { type: String, required: true }, + + // Provider is simply the name attached to the authentication mode. In the + // case of a locally provided profile, this will simply be `local`, or a + // social provider which for Facebook would just be `facebook`. provider: { type: String, required: true } - }], - roles: { - type: [{type: String, enum: ['admin', 'moderator']}] - } + }, { + _id: false + })], + + // Roles provides an array of roles (as strings) that is associated with a + // user. + roles: [String] }, { + + // This will ensure that we have proper timestamps available on this model. timestamps: { createdAt: 'created_at', updatedAt: 'updated_at' @@ -84,6 +124,13 @@ UserSchema.options.toObject.transform = (doc, ret, options) => { return ret; }; +// Create the User model. +const UserModel = mongoose.model('User', UserSchema); + +// UserService is the interface for the application to interact with the +// UserModel through. +const UserService = module.exports = {}; + /** * Finds a user given their email address that we have for them in the system * and ensures that the retuned user matches the password passed in as well. @@ -91,13 +138,12 @@ UserSchema.options.toObject.transform = (doc, ret, options) => { * @param {string} password - password to match against the found user * @param {Function} done [description] */ -UserSchema.statics.findLocalUser = function(email, password) { - +UserService.findLocalUser = (email, password) => { if (!email || typeof email !== 'string') { return Promise.reject('email is required for findLocalUser'); } - return User.findOne({ + return UserModel.findOne({ profiles: { $elemMatch: { id: email.toLowerCase(), @@ -134,13 +180,13 @@ UserSchema.statics.findLocalUser = function(email, password) { * @param {String} srcUserID id of the user to which is the source of the merge * @return {Promise} resolves when the users are merged */ -UserSchema.statics.mergeUsers = function(dstUserID, srcUserID) { +UserService.mergeUsers = (dstUserID, srcUserID) => { let srcUser, dstUser; return Promise .all([ - User.findOne({id: dstUserID}).exec(), - User.findOne({id: srcUserID}).exec() + UserModel.findOne({id: dstUserID}).exec(), + UserModel.findOne({id: srcUserID}).exec() ]) .then((users) => { dstUser = users[0]; @@ -161,8 +207,8 @@ UserSchema.statics.mergeUsers = function(dstUserID, srcUserID) { * @param {Object} profile - User social/external profile * @param {Function} done [description] */ -UserSchema.statics.findOrCreateExternalUser = function(profile) { - return User +UserService.findOrCreateExternalUser = (profile) => { + return UserModel .findOne({ profiles: { $elemMatch: { @@ -177,7 +223,7 @@ UserSchema.statics.findOrCreateExternalUser = function(profile) { } // The user was not found, lets create them! - user = new User({ + user = new UserModel({ displayName: profile.displayName, roles: [], profiles: [ @@ -192,7 +238,7 @@ UserSchema.statics.findOrCreateExternalUser = function(profile) { }); }; -UserSchema.statics.changePassword = function(id, password) { +UserService.changePassword = (id, password) => { return new Promise((resolve, reject) => { bcrypt.hash(password, SALT_ROUNDS, (err, hashedPassword) => { if (err) { @@ -203,7 +249,7 @@ UserSchema.statics.changePassword = function(id, password) { }); }) .then((hashedPassword) => { - return User.update({id}, { + return UserModel.update({id}, { $inc: {__v: 1}, $set: { password: hashedPassword @@ -217,9 +263,9 @@ UserSchema.statics.changePassword = function(id, password) { * @param {Array} users Users to create * @return {Promise} Resolves with the users that were created */ -UserSchema.statics.createLocalUsers = function(users) { +UserService.createLocalUsers = (users) => { return Promise.all(users.map((user) => { - return User + return UserService .createLocalUser(user.email, user.password, user.displayName); })); }; @@ -231,7 +277,7 @@ UserSchema.statics.createLocalUsers = function(users) { * @param {String} displayName name of the display user * @param {Function} done callback */ -UserSchema.statics.createLocalUser = function(email, password, displayName) { +UserService.createLocalUser = (email, password, displayName) => { if (!email) { return Promise.reject('email is required'); } @@ -252,7 +298,7 @@ UserSchema.statics.createLocalUser = function(email, password, displayName) { return reject(err); } - let user = new User({ + let user = new UserModel({ displayName: displayName, password: hashedPassword, roles: [], @@ -280,8 +326,8 @@ UserSchema.statics.createLocalUser = function(email, password, displayName) { * @param {String} id id of a user * @param {Function} done callback after the operation is complete */ -UserSchema.statics.disableUser = function(id) { - return User.update({ +UserService.disableUser = (id) => { + return UserModel.update({ id: id }, { $set: { @@ -295,8 +341,8 @@ UserSchema.statics.disableUser = function(id) { * @param {String} id id of a user * @param {Function} done callback after the operation is complete */ -UserSchema.statics.enableUser = function(id) { - return User.update({ +UserService.enableUser = (id) => { + return UserModel.update({ id: id }, { $set: { @@ -311,8 +357,16 @@ UserSchema.statics.enableUser = function(id) { * @param {String} role role to add * @param {Function} done callback after the operation is complete */ -UserSchema.statics.addRoleToUser = function(id, role) { - return User.update({ +UserService.addRoleToUser = (id, role) => { + + // Check to see if the user role is in the allowable set of roles. + if (USER_ROLES.indexOf(role) === -1) { + + // User role is not supported! Error out here. + return Promise.reject(new Error(`role ${role} is not supported`)); + } + + return UserModel.update({ id: id }, { $addToSet: { @@ -327,8 +381,8 @@ UserSchema.statics.addRoleToUser = function(id, role) { * @param {String} role role to remove * @param {Function} done callback after the operation is complete */ -UserSchema.statics.removeRoleFromUser = function(id, role) { - return User.update({ +UserService.removeRoleFromUser = (id, role) => { + return UserModel.update({ id: id }, { $pull: { @@ -341,16 +395,16 @@ UserSchema.statics.removeRoleFromUser = function(id, role) { * Finds a user with the id. * @param {String} id user id (uuid) */ -UserSchema.statics.findById = function(id) { - return User.findOne({id}); +UserService.findById = (id) => { + return UserModel.findOne({id}); }; /** * Finds users in an array of idd. * @param {Array} ids array of user identifiers (uuid) */ -UserSchema.statics.findByIdArray = function(ids) { - return User.find({ +UserService.findByIdArray = (ids) => { + return UserModel.find({ 'id': {$in: ids} }); }; @@ -359,14 +413,14 @@ UserSchema.statics.findByIdArray = function(ids) { * Creates a JWT from a user email. Only works for local accounts. * @param {String} email of the local user */ -UserSchema.statics.createJWT = function (email) { +UserService.createJWT = function (email) { if (!email || typeof email !== 'string') { return Promise.reject('email is required when creating a JWT for resetting passord'); } email = email.toLowerCase(); - return this.findOne({profiles: {$elemMatch: {id: email}}}) + return UserModel.findOne({profiles: {$elemMatch: {id: email}}}) .then(user => { if (user === null) { @@ -384,7 +438,7 @@ UserSchema.statics.createJWT = function (email) { * verifies a jwt and returns the associated user * @param {String} token the JSON Web Token to verify */ -UserSchema.statics.verifyToken = function (token) { +UserService.verifyToken = function (token) { return new Promise((resolve, reject) => { jwt.verify(token, process.env.TALK_SESSION_SECRET, (error, decoded) => { if (error) { @@ -404,7 +458,36 @@ UserSchema.statics.verifyToken = function (token) { }); }; -const User = mongoose.model('User', UserSchema); +/** + * Finds a user using a value which gets compared using a prefix match against + * the user's email address and/or their display name. + * @param {String} value value to search by + * @return {Promise} + */ +UserService.search = (value) => { + return UserModel.find({ + $or: [ -module.exports = User; -module.exports.Schema = UserSchema; + // Search by a prefix match on the displayName. + { + 'displayName': { + $regex: new RegExp(`^${value}`), + $options: 'i' + } + }, + + // Search by a prefix match on the email address. + { + 'profiles': { + $elemMatch: { + id: { + $regex: new RegExp(`^${value}`), + $options: 'i' + }, + provider: 'local' + } + } + } + ] + }); +}; diff --git a/routes/api/user/index.js b/routes/api/user/index.js index c9b497659..4f9110b8f 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -2,6 +2,11 @@ const express = require('express'); const router = express.Router(); const User = require('../../../models/user'); const mailer = require('../../../services/mailer'); +const ejs = require('ejs'); +const fs = require('fs'); +const path = require('path'); +const resetEmailFile = fs.readFileSync(path.resolve(__dirname, '../../../views/password-reset-email.ejs')); +const resetEmailTemplate = ejs.compile(resetEmailFile.toString()); router.get('/', (req, res, next) => { const { @@ -12,28 +17,9 @@ router.get('/', (req, res, next) => { limit = 50 // Total Per Page } = req.query; - let q = { - $or: [ - { - 'displayName': { - $regex: new RegExp(`^${value}`), - $options: 'i' - }, - 'profiles': { - $elemMatch: { - id: { - $regex: new RegExp(`^${value}`), - $options: 'i' - }, - provider: 'local' - } - } - } - ] - }; - Promise.all([ - User.find(q) + User + .search(value) .sort({[field]: (asc === 'true') ? 1 : -1}) .skip((page - 1) * limit) .limit(limit), @@ -64,11 +50,12 @@ router.get('/', (req, res, next) => { }); router.post('/:user_id/role', (req, res, next) => { - User.addRoleToUser(req.params.user_id, req.body.role) - .then(role => { - res.send(role); - }) - .catch(next); + User + .addRoleToUser(req.params.user_id, req.body.role) + .then(role => { + res.send(role); + }) + .catch(next); }); /** @@ -110,9 +97,11 @@ router.post('/request-password-reset', (req, res, next) => { subject: 'Password Reset Requested - Talk', from: 'coralcore@mozillafoundation.org', to: email, - html: `

We received a request to reset your password. If you did not request this change, you can ignore this email. - If you did, please click here to reset password.

- ${process.env.NODE_ENV === 'production' ? '' : `

${token}

`}` + html: resetEmailTemplate({ + token, + // probably more clear to explicitly pass this + rootURL: process.env.TALK_ROOT_URL + }) }; return mailer.sendSimple(options); }) diff --git a/views/password-reset-email.ejs b/views/password-reset-email.ejs new file mode 100644 index 000000000..0637b6bb2 --- /dev/null +++ b/views/password-reset-email.ejs @@ -0,0 +1,6 @@ + +

We received a request to reset your password. If you did not request this change, you can ignore this email.
+If you did, please click here to reset password.

+<% if (process.env.NODE_ENV !== 'production') { %> +

<%= token %>

+<% } %> From 5b35d3b61dd454fa73c593a33aae257416cee4d9 Mon Sep 17 00:00:00 2001 From: riley Date: Wed, 16 Nov 2016 15:38:42 -0700 Subject: [PATCH 11/15] update to TALK_SMTP_CONNECTION_URL --- services/mailer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/mailer.js b/services/mailer.js index d90d04071..347e7a63d 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,14 +1,14 @@ const nodemailer = require('nodemailer'); const debug = require('debug')('talk:mail'); -if (!process.env.TALK_SMTP_STRING) { +if (!process.env.TALK_SMTP_CONNECTION_URL) { debug('\n///////////////////////////////////////////////////////////////\n' + - '/// TALK_SENDGRID_APIKEY should be defined if you would ///\n' + + '/// TALK_SMTP_CONNECTION_URL should be defined if you would ///\n' + '/// like to send password reset emails from Talk ///\n' + '///////////////////////////////////////////////////////////////'); } -const transporter = nodemailer.createTransport(process.env.TALK_SMTP_STRING); +const transporter = nodemailer.createTransport(process.env.TALK_SMTP_CONNECTION_URL); const mailer = { /** From 3c835c1ce5a3ef7eb7e4ff7bf37d409615faa1a0 Mon Sep 17 00:00:00 2001 From: riley Date: Wed, 16 Nov 2016 15:59:44 -0700 Subject: [PATCH 12/15] update from address --- routes/api/user/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 4f9110b8f..b9c6efa3b 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -95,7 +95,7 @@ router.post('/request-password-reset', (req, res, next) => { .then(token => { const options = { subject: 'Password Reset Requested - Talk', - from: 'coralcore@mozillafoundation.org', + from: 'noreply@coralproject.net', to: email, html: resetEmailTemplate({ token, From 04decde9fcf032501507d98862451ffb1420f12f Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 17 Nov 2016 10:31:46 -0700 Subject: [PATCH 13/15] throw instead of log errors. rename for clarity --- models/user.js | 9 ++++----- routes/api/user/index.js | 4 ++-- services/mailer.js | 3 +-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/models/user.js b/models/user.js index bbf8f21e6..01dbb37a9 100644 --- a/models/user.js +++ b/models/user.js @@ -2,7 +2,6 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); -const debug = require('debug')('talk:user_model'); // SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run // through during the salting process. @@ -15,7 +14,7 @@ const USER_ROLES = [ ]; if (!process.env.TALK_SESSION_SECRET) { - debug('\n////////////////////////////////////////////////////////////\n' + + throw new Error('\n////////////////////////////////////////////////////////////\n' + '/// TALK_SESSION_SECRET must be defined to encode ///\n' + '/// JSON Web Tokens and other auth functionality ///\n' + '////////////////////////////////////////////////////////////'); @@ -413,7 +412,7 @@ UserService.findByIdArray = (ids) => { * Creates a JWT from a user email. Only works for local accounts. * @param {String} email of the local user */ -UserService.createJWT = function (email) { +UserService.createPasswordResetToken = function (email) { if (!email || typeof email !== 'string') { return Promise.reject('email is required when creating a JWT for resetting passord'); } @@ -438,7 +437,7 @@ UserService.createJWT = function (email) { * verifies a jwt and returns the associated user * @param {String} token the JSON Web Token to verify */ -UserService.verifyToken = function (token) { +UserService.verifyPasswordResetToken = function (token) { return new Promise((resolve, reject) => { jwt.verify(token, process.env.TALK_SESSION_SECRET, (error, decoded) => { if (error) { @@ -454,7 +453,7 @@ UserService.verifyToken = function (token) { * and make an entry if it does not exist. * reject if entry already exists. */ - return this.findOne({id: decoded.userId}); + return this.findById(decoded.userId); }); }; diff --git a/routes/api/user/index.js b/routes/api/user/index.js index b9c6efa3b..4b00cf785 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -66,7 +66,7 @@ router.post('/:user_id/role', (req, res, next) => { router.post('/update-password', (req, res, next) => { const {token, password} = req.body; - User.verifyToken(token) + User.verifyPasswordResetToken(token) .then(user => { return User.changePassword(user.id, password); }) @@ -91,7 +91,7 @@ router.post('/request-password-reset', (req, res, next) => { } User - .createJWT(email) + .createPasswordResetToken(email) .then(token => { const options = { subject: 'Password Reset Requested - Talk', diff --git a/services/mailer.js b/services/mailer.js index 347e7a63d..cdb23f370 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,8 +1,7 @@ const nodemailer = require('nodemailer'); -const debug = require('debug')('talk:mail'); if (!process.env.TALK_SMTP_CONNECTION_URL) { - debug('\n///////////////////////////////////////////////////////////////\n' + + throw new Error('\n///////////////////////////////////////////////////////////////\n' + '/// TALK_SMTP_CONNECTION_URL should be defined if you would ///\n' + '/// like to send password reset emails from Talk ///\n' + '///////////////////////////////////////////////////////////////'); From 869d871157377df225a26c4b8c2609f85c4d92bd Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 17 Nov 2016 11:25:19 -0700 Subject: [PATCH 14/15] always return 204 for password reset. update swagger file --- models/user.js | 4 +++- routes/api/user/index.js | 8 +++++++- swagger.yaml | 32 +++++++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/models/user.js b/models/user.js index 01dbb37a9..23cb54bae 100644 --- a/models/user.js +++ b/models/user.js @@ -423,7 +423,9 @@ UserService.createPasswordResetToken = function (email) { .then(user => { if (user === null) { - return Promise.reject(`Uh oh! We've never heard of ${email}. Maybe there's a typo in there?`); + // since we don't want to reveal that the email does/doesn't exist + // just go ahead and resolve the Promise with null and check in the endpoint + return Promise.resolve(null); } const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 4b00cf785..3faab9db3 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -93,6 +93,10 @@ router.post('/request-password-reset', (req, res, next) => { User .createPasswordResetToken(email) .then(token => { + if (token === null) { + return Promise.resolve('the email was not found in the db.'); + } + const options = { subject: 'Password Reset Requested - Talk', from: 'noreply@coralproject.net', @@ -106,7 +110,9 @@ router.post('/request-password-reset', (req, res, next) => { return mailer.sendSimple(options); }) .then(() => { - res.json({success: true}); + // we want to send a 204 regardless of the user being found in the db + // if we fail on missing emails, it would reveal if people are registered or not. + res.status(204).send('OK'); }) .catch(error => { const errorMsg = typeof error === 'string' ? error : error.message; diff --git a/swagger.yaml b/swagger.yaml index ba945e2e1..16ae9a960 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -162,6 +162,36 @@ paths: responses: 204: description: OK + + /user/request-password-reset: + post: + tags: + - Users + description: trigger a reset password email. sends a success code whether email was found or no. + responses: + 204: + description: OK + + /user/update-password: + post: + tags: + - Users + description: Update existing user password + parameters: + - name: token + type: string + in: body + description: JSON Web token taken taken from emailed link + required: true + - name: password + type: string + in: body + description: new password to be settings + required: true + responses: + 204: + description: OK + /asset: get: tags: @@ -276,7 +306,7 @@ definitions: description: A summary of the asset, inferred on initial load. section: type: string - description: The section the asset is in. + description: The section the asset is in. subsection: type: string description: The subsection that the asset is in. From 145bb68681ddefbf346316fbb2333d258eb18aa1 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 17 Nov 2016 11:31:27 -0700 Subject: [PATCH 15/15] don't use this keyword in UserService methods --- models/user.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/user.js b/models/user.js index 23cb54bae..45db2adf8 100644 --- a/models/user.js +++ b/models/user.js @@ -439,7 +439,7 @@ UserService.createPasswordResetToken = function (email) { * verifies a jwt and returns the associated user * @param {String} token the JSON Web Token to verify */ -UserService.verifyPasswordResetToken = function (token) { +UserService.verifyPasswordResetToken = token => { return new Promise((resolve, reject) => { jwt.verify(token, process.env.TALK_SESSION_SECRET, (error, decoded) => { if (error) { @@ -455,7 +455,7 @@ UserService.verifyPasswordResetToken = function (token) { * and make an entry if it does not exist. * reject if entry already exists. */ - return this.findById(decoded.userId); + return UserService.findById(decoded.userId); }); };