From a7680ae5d928bb32a95b0b12f77cd465148e84bb Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 3 Jan 2017 17:35:58 -0700 Subject: [PATCH 1/6] Initial pass at email confirmation --- bin/cli-jobs | 8 +- bin/cli-serve | 9 +- client/coral-framework/actions/auth.js | 2 +- client/coral-framework/actions/user.js | 6 +- client/coral-framework/reducers/user.js | 3 +- docs/swagger.yaml | 2 +- models/action.js | 2 +- models/setting.js | 21 +--- models/user.js | 114 +++++++++-------- routes/api/account/index.js | 118 ++++++++++++++++++ routes/api/auth/index.js | 4 + routes/api/index.js | 1 + routes/api/users/index.js | 110 ++-------------- scripts/pree2e.sh | 8 +- services/kue.js | 84 +++++++++++-- services/mailer.js | 110 ++++++++++++---- services/mongoose.js | 42 ++++++- services/passport.js | 47 ++++++- services/scraper.js | 64 ++++------ tests/routes/api/auth/index.js | 15 ++- .../password-reset.ejs} | 4 - views/email/password-reset.txt.ejs | 5 + views/password-reset.ejs | 4 +- 23 files changed, 498 insertions(+), 285 deletions(-) create mode 100644 routes/api/account/index.js rename views/{password-reset-email.ejs => email/password-reset.ejs} (58%) create mode 100644 views/email/password-reset.txt.ejs diff --git a/bin/cli-jobs b/bin/cli-jobs index 60a7b8efb..879927197 100755 --- a/bin/cli-jobs +++ b/bin/cli-jobs @@ -6,6 +6,7 @@ const program = require('commander'); const scraper = require('../services/scraper'); +const mailer = require('../services/mailer'); const util = require('../util'); const mongoose = require('../services/mongoose'); const kue = require('../services/kue'); @@ -19,13 +20,16 @@ util.onshutdown([ */ function processJobs() { - // Start the processor. + // Start the scraper processor. scraper.process(); + // Start the mail processor. + mailer.process(); + // The scraper only needs to shutdown when the scraper has actually been // started. util.onshutdown([ - () => scraper.shutdown() + () => kue.Task.shutdown() ]); } diff --git a/bin/cli-serve b/bin/cli-serve index 05fe6efc7..5095c6362 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -5,6 +5,8 @@ const debug = require('debug')('talk:server'); const http = require('http'); const init = require('../init'); const scraper = require('../services/scraper'); +const mailer = require('../services/mailer'); +const kue = require('../services/kue'); const mongoose = require('../services/mongoose'); const util = require('../util'); @@ -119,15 +121,18 @@ startApp(); // Enable job processing on the thread if enabled. if (program.jobs) { - // Start the processor. + // Start the scraper processor. scraper.process(); + + // Start the mail processor. + mailer.process(); } // Define a safe shutdown function to call in the event we need to shutdown // because the node hooks are below which will interrupt the shutdown process. // Shutdown the mongoose connection, the app server, and the scraper. util.onshutdown([ - () => program.jobs ? scraper.shutdown() : null, + () => program.jobs ? kue.Task.shutdown() : null, () => mongoose.disconnect(), () => server.close() ]); diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index c5390607c..eebc0d3ce 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -92,7 +92,7 @@ const forgotPassowordFailure = () => ({type: actions.FETCH_FORGOT_PASSWORD_FAILU export const fetchForgotPassword = email => dispatch => { dispatch(forgotPassowordRequest(email)); - coralApi('/users/request-password-reset', {method: 'POST', body: {email}}) + coralApi('/account/password/reset', {method: 'POST', body: {email}}) .then(() => dispatch(forgotPassowordSuccess())) .catch(error => dispatch(forgotPassowordFailure(error))); }; diff --git a/client/coral-framework/actions/user.js b/client/coral-framework/actions/user.js index 0ee8659d8..b4b7b7896 100644 --- a/client/coral-framework/actions/user.js +++ b/client/coral-framework/actions/user.js @@ -14,10 +14,10 @@ const saveBioFailure = error => ({type: actions.SAVE_BIO_FAILURE, error}); export const saveBio = (user_id, formData) => dispatch => { dispatch(saveBioRequest()); - coralApi(`/users/${user_id}/bio`, {method: 'PUT', body: formData}) - .then(({settings}) => { + coralApi('/account/bio', {method: 'PUT', body: formData}) + .then(() => { dispatch(addNotification('success', lang.t('successBioUpdate'))); - dispatch(saveBioSuccess(settings)); + dispatch(saveBioSuccess(formData)); }) .catch(error => dispatch(saveBioFailure(error))); }; diff --git a/client/coral-framework/reducers/user.js b/client/coral-framework/reducers/user.js index bd5f78e87..3970aa679 100644 --- a/client/coral-framework/reducers/user.js +++ b/client/coral-framework/reducers/user.js @@ -31,8 +31,7 @@ export default function user (state = initialState, action) { case authActions.FETCH_SIGNIN_FACEBOOK_FAILURE: return initialState; case actions.SAVE_BIO_SUCCESS: - return state - .set('settings', action.settings); + return state.set('settings', action.settings); case actions.COMMENTS_BY_USER_SUCCESS: return state.set('myComments', action.comments); case assetActions.MULTIPLE_ASSETS_SUCCESS: diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 8fe8627b0..554afb29e 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -583,7 +583,7 @@ paths: description: The user that has been created. schema: $ref: '#/definitions/User' - /users/update-password: + /account/password/reset: post: parameters: - name: body diff --git a/models/action.js b/models/action.js index ee698029d..8e6634072 100644 --- a/models/action.js +++ b/models/action.js @@ -13,7 +13,7 @@ const ActionSchema = new Schema({ item_type: String, item_id: String, user_id: String, - metadata: Object, //Holds arbitrary metadata about the action. + metadata: Schema.Types.Mixed }, { timestamps: { createdAt: 'created_at', diff --git a/models/setting.js b/models/setting.js index e9e38f463..9e71e36aa 100644 --- a/models/setting.js +++ b/models/setting.js @@ -1,7 +1,6 @@ const mongoose = require('../services/mongoose'); const Schema = mongoose.Schema; const _ = require('lodash'); -const cache = require('../services/cache'); const WordlistSchema = new Schema({ banned: [String], @@ -53,6 +52,10 @@ const SettingSchema = new Schema({ charCountEnable: { type: Boolean, default: false + }, + requireEmailConfirmation: { + type: Boolean, + default: false } }, { timestamps: { @@ -121,19 +124,11 @@ const SettingService = module.exports = {}; */ const selector = {id: '1'}; -/** - * Cache expiry time in seconds for when the cached entry of the settings object - * expires. 2 minutes. - */ -const EXPIRY_TIME = 60 * 2; - /** * Gets the entire settings record and sends it back * @return {Promise} settings the whole settings record */ -SettingService.retrieve = () => cache.wrap('settings', EXPIRY_TIME, () => { - return Setting.findOne(selector); -}).then((setting) => new Setting(setting)); +SettingService.retrieve = () => Setting.findOne(selector); /** * This will update the settings object with whatever you pass in @@ -146,12 +141,6 @@ SettingService.update = (settings) => Setting.findOneAndUpdate(selector, { upsert: true, new: true, setDefaultsOnInsert: true -}).then((settings) => { - - // Invalidate the settings cache. - return cache - .set('settings', settings, EXPIRY_TIME) - .then(() => settings); }); /** diff --git a/models/user.js b/models/user.js index 7df713337..0f8fa3e79 100644 --- a/models/user.js +++ b/models/user.js @@ -4,7 +4,6 @@ const _ = require('lodash'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const Action = require('./action'); - const Comment = require('./comment'); // SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run @@ -31,6 +30,37 @@ if (process.env.NODE_ENV === 'test' && !process.env.TALK_SESSION_SECRET) { throw new Error('TALK_SESSION_SECRET must be defined to encode JSON Web Tokens and other auth functionality'); } +// ProfileSchema is the mongoose schema defined as the representation of a +// User's profile stored in MongoDB. +const ProfileSchema = 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 + }, + + // Metadata provides a place to put provider specific details. An example of + // something that could be stored here is the `metadata.confirmed_at` could be + // used by the `local` provider to indicate when the email address was + // confirmed. + metadata: { + type: mongoose.Schema.Types.Mixed + } +}, { + _id: false +}); + // UserSchema is the mongoose schema defined as the representation of a User in // MongoDB. const UserSchema = new mongoose.Schema({ @@ -60,26 +90,7 @@ const UserSchema = new mongoose.Schema({ // 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 - } - }, { - _id: false - })], + profiles: [ProfileSchema], // Roles provides an array of roles (as strings) that is associated with a // user. @@ -499,45 +510,43 @@ UserService.createPasswordResetToken = function (email) { email = email.toLowerCase(); return UserModel.findOne({profiles: {$elemMatch: {id: email}}}) - .then(user => { + .then((user) => { + if (!user) { - if (user === null) { - - // 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); + // 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; } - const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; - const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); + const payload = { + jti: uuid.v4(), + email, + userId: user.id, + version: user.__v + }; - return token; + return jwt.sign(payload, process.env.TALK_SESSION_SECRET, {algorithm: 'HS256'}, { + expiresIn: '1d' + }); }); }; /** - * verifies a jwt and returns the associated user + * Verifies a jwt and returns the associated user. * @param {String} token the JSON Web Token to verify */ UserService.verifyPasswordResetToken = token => { return new Promise((resolve, reject) => { - jwt.verify(token, process.env.TALK_SESSION_SECRET, (error, decoded) => { - if (error) { - return reject(error); + jwt.verify(token, process.env.TALK_SESSION_SECRET, (err, decoded) => { + if (err) { + return reject(err); } 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 UserService.findById(decoded.userId); - }); + .then(decoded => UserService.findById(decoded.userId)); }; /** @@ -594,18 +603,13 @@ UserService.all = () => { * Adds a new User bio * @return {Promise} */ - -UserService.addBio = (id, bio) => ( - UserModel.findOneAndUpdate({ - id - }, { - $set: { - 'settings.bio': bio - } - }, { - new: true - }) -); +UserService.addBio = (id, bio) => UserModel.update({ + id +}, { + $set: { + 'settings.bio': bio + } +}); /** * Add an action to the user. diff --git a/routes/api/account/index.js b/routes/api/account/index.js new file mode 100644 index 000000000..b975ba284 --- /dev/null +++ b/routes/api/account/index.js @@ -0,0 +1,118 @@ +const express = require('express'); +const router = express.Router(); +const User = require('../../../models/user'); +const mailer = require('../../../services/mailer'); +const authorization = require('../../../middleware/authorization'); + +router.get('/', authorization.needed(), (req, res, next) => { + res.json(req.user); +}); + +/** + * 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('/password/reset', (req, res, next) => { + const {email} = req.body; + + if (!email) { + return next('you must submit an email when requesting a password.'); + } + + User + .createPasswordResetToken(email) + .then((token) => { + + // Check to see if the token isn't defined. + if (!token) { + + // As it isn't, don't send any emails! + return; + } + + return mailer.sendSimple({ + app: req.app, // needed to render the templates. + template: 'email/password-reset', // needed to know which template to render! + locals: { // specifies the template locals. + token, + rootURL: process.env.TALK_ROOT_URL + }, + subject: 'Password Reset Requested - Talk', + to: email + }); + }) + .then(() => { + + // 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).end(); + }) + .catch((err) => { + next(err); + }); +}); + +// ErrPasswordTooShort is returned when the password length is too short. +const ErrPasswordTooShort = new Error('password must be at least 8 characters'); +ErrPasswordTooShort.status = 400; + +// ErrMissingToken is returned in the event that the password reset is requested +// without a token. +const ErrMissingToken = new Error('token is required'); +ErrMissingToken.status = 400; + +/** + * 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.put('/password/reset', (req, res, next) => { + + const { + token, + password + } = req.body; + + if (!token) { + return next(ErrMissingToken); + } + + if (!password || password.length < 8) { + return next(ErrPasswordTooShort); + } + + User.verifyPasswordResetToken(token) + .then(user => { + return User.changePassword(user.id, password); + }) + .then(() => { + res.status(204).end(); + }) + .catch(error => { + console.error(error); + + next(authorization.ErrNotAuthorized); + }); +}); + +router.put('/bio', authorization.needed(), (req, res, next) => { + + const { + bio + } = req.body; + + if (!bio) { + return next(new Error('You must submit a new bio')); + } + + User + .addBio(req.user.id, bio) + .then(() => { + res.status(204).end(); + }) + .catch((err) => { + next(err); + }); +}); + +module.exports = router; diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index bcbfcc27e..c798b0ebc 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -31,6 +31,10 @@ router.delete('/', authorization.needed(), (req, res) => { }); }); +//============================================================================== +// PASSPORT ROUTES +//============================================================================== + /** * This sends back the user data as JSON. */ diff --git a/routes/api/index.js b/routes/api/index.js index 316d284e0..120fd47ec 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -17,6 +17,7 @@ router.use('/actions', authorization.needed(), require('./actions')); router.use('/auth', require('./auth')); router.use('/stream', require('./stream')); router.use('/users', require('./users')); +router.use('/account', require('./account')); // Bind the kue handler to the /kue path. router.use('/kue', authorization.needed('admin'), require('../../services/kue').kue.app); diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 1ce2e0e26..8dc48d5dc 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -1,12 +1,6 @@ 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()); const authorization = require('../../../middleware/authorization'); router.get('/', authorization.needed('admin'), (req, res, next) => { @@ -50,19 +44,22 @@ router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => router.post('/:user_id/status', (req, res, next) => { User .setStatus(req.params.user_id, req.body.status, req.body.comment_id) - .then(status => { - res.json(status); + .then((status) => { + res.status(201).json(status); }) .catch(next); }); -router.post('/', (req, res, next) => { - const {email, password, displayName} = req.body; +router.post('/', authorization.needed('admin'), (req, res, next) => { + const { + email, + password, + displayName + } = req.body; User .createLocalUser(email, password, displayName) .then(user => { - res.status(201).json(user); }) .catch(err => { @@ -70,96 +67,7 @@ router.post('/', (req, res, next) => { }); }); -const ErrPasswordTooShort = new Error('password must be at least 8 characters'); -ErrPasswordTooShort.status = 400; - -/** - * 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; - - if (!password || password.length < 8) { - return next(ErrPasswordTooShort); - } - - User.verifyPasswordResetToken(token) - .then(user => { - return User.changePassword(user.id, password); - }) - .then(() => { - res.status(204).end(); - }) - .catch(error => { - console.error(error); - - next(authorization.ErrNotAuthorized); - }); -}); - -/** - * 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; - - if (!email) { - return next('you must submit an email when requesting a password.'); - } - - 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: process.env.TALK_SMTP_FROM_ADDRESS, - to: email, - html: resetEmailTemplate({ - token, - - // probably more clear to explicitly pass this - rootURL: process.env.TALK_ROOT_URL - }) - }; - return mailer.sendSimple(options); - }) - .then(() => { - - // 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).end(); - }) - .catch((err) => { - next(err); - }); -}); - -router.put('/:user_id/bio', (req, res, next) => { - const {user_id} = req.params; - const {bio} = req.body; - - if (!bio) { - return next('You must submit a new bio'); - } - - User - .addBio(user_id, bio) - .then(user => { - res.json(user); - }) - .catch((err) => { - next(err); - }); -}); - -router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { +router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { const { action_type, metadata diff --git a/scripts/pree2e.sh b/scripts/pree2e.sh index e79addc2e..d8624e81d 100755 --- a/scripts/pree2e.sh +++ b/scripts/pree2e.sh @@ -4,12 +4,12 @@ selenium-standalone install # Creating Admin Test User -{ echo admin@test.com; echo test; echo test; echo Admin Test User; echo admin;} | dotenv ./bin/cli-users create +{ echo admin@test.com; echo test; echo test; echo Admin Test User; echo admin;} | ./bin/cli-users create # Creating Moderator Test User -{ echo moderator@test.com; echo test; echo test; echo Moderator Test User; echo moderator;} | dotenv ./bin/cli-users create +{ echo moderator@test.com; echo test; echo test; echo Moderator Test User; echo moderator;} | ./bin/cli-users create # Creating Commenter Test User -{ echo commenter@test.com; echo test; echo test; echo Commenter Test User; echo ;} | dotenv ./bin/cli-users create +{ echo commenter@test.com; echo test; echo test; echo Commenter Test User; echo ;} | ./bin/cli-users create -npm start +npm start & diff --git a/services/kue.js b/services/kue.js index e2229d424..e9fbe43e8 100644 --- a/services/kue.js +++ b/services/kue.js @@ -1,11 +1,79 @@ -const kue = require('kue'); +const debug = require('debug')('talk:services:kue'); const redis = require('./redis'); -module.exports = { - queue: kue.createQueue({ - redis: { - createClientFactory: () => redis.createClient() - } - }), - kue +module.exports = {}; + +const kue = module.exports.kue = require('kue'); + +// Note that unlike what the name createQueue suggests, it currently returns a +// singleton Queue instance. So you can configure and use only a single Queue +// object within your node.js process. +const Queue = module.exports.queue = kue.createQueue({ + redis: { + createClientFactory: () => redis.createClient() + } +}); + +module.exports.Task = class Task { + + constructor({name, attempts = 3, delay = 1000}) { + this.name = name; + this.attempts = attempts; + this.delay = delay; + } + + /** + * Add a new job to the queue. + */ + create(data) { + + debug(`Creating new job for Queue[${this.name}]`); + + return new Promise((resolve, reject) => { + let job = Queue + .create(this.name, data) + .attempts(this.attempts) + .delay(this.delay) + .backoff({type: 'exponential'}) + .save((err) => { + if (err) { + return reject(err); + } + + debug(`Job[${job.id}] created on Queue[${this.name}]`); + + return resolve(job); + }); + }); + } + + /** + * Process jobs for the queue. + */ + process(callback) { + return Queue.process(this.name, callback); + } + + /** + * Shutdown running jobs. + */ + static shutdown() { + + debug('Shutting down the Queue'); + + return new Promise((resolve, reject) => { + + // Shutdown and give the queue 5 seconds to shutdown before we start + // killing jobs. + Queue.shutdown(5000, (err) => { + if (err) { + return reject(err); + } + + debug('Queue shut down.'); + + resolve(); + }); + }); + } }; diff --git a/services/mailer.js b/services/mailer.js index 9ff7291d4..6fef876d6 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,4 +1,6 @@ +const debug = require('debug')('talk:services:mailer'); const nodemailer = require('nodemailer'); +const kue = require('./kue'); const smtpRequiredProps = [ 'TALK_SMTP_FROM_ADDRESS', @@ -7,11 +9,9 @@ const smtpRequiredProps = [ 'TALK_SMTP_HOST' ]; -smtpRequiredProps.forEach(prop => { - if (!process.env[prop]) { - console.error(`process.env.${prop} should be defined if you would like to send password reset emails from Talk`); - } -}); +if (smtpRequiredProps.some(prop => !process.env[prop])) { + console.error(`${smtpRequiredProps.join(', ')} should be defined in the environment if you would like to send password reset emails from Talk`); +} const options = { host: process.env.TALK_SMTP_HOST, @@ -29,29 +29,89 @@ if (process.env.TALK_SMTP_PORT) { const defaultTransporter = nodemailer.createTransport(options); -const mailer = { +const mailer = module.exports = { /** - * sendSimple - * - * @param {Object} {from, to, subject, text = '', html = ''} - * @returns - */ - sendSimple({from, to, subject, text = '', html = '', transporter = defaultTransporter}) { - 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'); - } + * Create the new Task kue. + */ + task: new kue.Task({ + name: 'mailer' + }), - return resolve(transporter.sendMail({from, to, subject, text, html})); + /** + * Render renders the template with the given locals and returns the rendered + * html/text. + */ + render(app, template, locals = {}) { + return new Promise((resolve, reject) => { + + // Render the template with the app.render method. + app.render(template, locals, (err, rendered) => { + if (err) { + return reject(err); + } + + return resolve(rendered); + }); + }); + }, + + sendSimple({app, template, locals, to, subject}) { + if (!to) { + return Promise.reject('sendSimple requires a comma-separated list of "to" addresses'); + } + + if (!subject) { + return Promise.reject('sendSimple requires a subject for the email'); + } + + return Promise.all([ + + // Render the HTML version of the email. + mailer.render(app, template, locals), + + // Render the TEXT version of the email. + mailer.render(app, `${template}.txt`, locals) + ]) + .then(([html, text]) => { + + // Create the job. + return mailer.task.create({ + title: 'Mail', + message: { + to, + subject, + text, + html + } + }); + }); + }, + + /** + * Start the queue processor for the mailer job. + */ + process() { + + debug(`Now processing ${mailer.task.name} jobs`); + + return mailer.task.process(({id, data}, done) => { + debug(`Starting to send mail for Job[${id}]`); + + // Set the `from` field. + data.message.from = process.env.TALK_SMTP_FROM_ADDRESS; + + // Actually send the email. + defaultTransporter.sendMail(data.message, (err) => { + if (err) { + debug(`Failed to send mail for Job[${id}]:`, err); + return done(err); + } + + debug(`Finished sending mail for Job[${id}]`); + return done(); + }); }); } -}; -module.exports = mailer; +}; diff --git a/services/mongoose.js b/services/mongoose.js index 03c87b937..1b9097760 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -1,21 +1,53 @@ const mongoose = require('mongoose'); const debug = require('debug')('talk:db'); +const queryDebuger = require('debug')('talk:db:query'); + +// Loading the formatter from Mongoose: +// +// https://github.com/Automattic/mongoose/blob/1a93d1f4d12e441e17ddf451e96fbc5f6e8f54b8/lib/drivers/node-mongodb-native/collection.js#L182 +// +// so we can wrap parameters. +const formatter = require('mongoose').Collection.prototype.$format; + +// Provide a newly wrapped debugQuery function which wraps the `debug` package. +function debugQuery(name, i, ...args) { + let functionCall = ['db', name, i].join('.'); + let _args = []; + for (let j = args.length - 1; j >= 0; --j) { + if (formatter(args[j]) || _args.length) { + _args.unshift(formatter(args[j])); + } + } + + let params = `(${_args.join(', ')})`; + + queryDebuger(functionCall + params); +} + const enabled = require('debug').enabled; -// Append '-test' to the db if node_env === 'test' -let url = process.env.TALK_MONGO_URL || 'mongodb://localhost/coral-talk'; +// Pull the mongo url out of the environment. +let url = process.env.TALK_MONGO_URL; -if (process.env.NODE_ENV === 'test') { - url += '-test'; +// Reset the mongo url in the event it hasn't been overrided and we are in a +// testing environment. Every new mongo instance comes with a test database by +// default, this is consistent with common testing and use case practices. +if (process.env.NODE_ENV === 'test' && !url) { + url = 'mongodb://localhost/test'; } // Use native promises mongoose.Promise = global.Promise; +// Check if debugging is enabled on the talk:db prefix. if (enabled('talk:db')) { - mongoose.set('debug', true); + + // Enable the mongoose debugger, here we wrap the similar print function + // provided by setting the debug parameter. + mongoose.set('debug', debugQuery); } +// Connect to the Mongo instance. mongoose.connect(url, (err) => { if (err) { throw err; diff --git a/services/passport.js b/services/passport.js index f709970dc..130de234d 100644 --- a/services/passport.js +++ b/services/passport.js @@ -1,5 +1,6 @@ const passport = require('passport'); const User = require('../models/user'); +const Setting = require('../models/setting'); const LocalStrategy = require('passport-local').Strategy; const FacebookStrategy = require('passport-facebook').Strategy; @@ -27,7 +28,7 @@ passport.deserializeUser((id, done) => { * @param {User} user the user to be validated * @param {Function} done the callback for the validation */ -function ValidateUserLogin(user, done) { +function ValidateUserLogin(loginProfile, user, done) { if (!user) { return done(new Error('user not found')); } @@ -36,7 +37,36 @@ function ValidateUserLogin(user, done) { return done(null, false, {message: 'Account disabled'}); } - return done(null, user); + // If the user isn't a local user (i.e., a social user). + if (loginProfile.provider !== 'local') { + return done(null, user); + } + + // The user is a local user, check if we need email confirmation. + return Setting.retrieve().then(({requireEmailConfirmation = false}) => { + + // If we have the requirement of checking that emails for users are + // verified, then we need to check the email address to ensure that it has + // been verified. + if (requireEmailConfirmation) { + + // Get the profile representing the local account. + let profile = user.profiles.find((profile) => profile.id === loginProfile.id); + + // This should never get to this point, if it does, don't let this past. + if (!profile) { + throw new Error('ID indicated by loginProfile is not on user object'); + } + + // If the profile doesn't have a metadata field, or it does not have a + // confirmed_at field, or that field is null, then send them back. + if (!profile.metadata || !profile.metadata.confirmed_at || profile.metadata.confirmed_at === null) { + return done(null, false, {message: `Email address ${loginProfile.id} not verified.`}); + } + } + + return done(null, user); + }); } //============================================================================== @@ -54,7 +84,12 @@ passport.use(new LocalStrategy({ return done(null, false, {message: 'Incorrect email/password combination'}); } - return ValidateUserLogin(user, done); + // Define the loginProfile being used to perform an additional + // verificaiton. + let loginProfile = {id: email, provider: 'local'}; + + // Validate the user login. + return ValidateUserLogin(loginProfile, user, done); }) .catch((err) => { done(err); @@ -70,9 +105,9 @@ if (process.env.TALK_FACEBOOK_APP_ID && process.env.TALK_FACEBOOK_APP_SECRET && }, (accessToken, refreshToken, profile, done) => { User .findOrCreateExternalUser(profile) - .then((user) => - ValidateUserLogin(user, done) - ) + .then((user) => { + return ValidateUserLogin(profile, user, done); + }) .catch((err) => { done(err); }); diff --git a/services/scraper.js b/services/scraper.js index c75a12750..6a3260757 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -1,7 +1,6 @@ const kue = require('./kue'); const debug = require('debug')('talk:services:scraper'); const Asset = require('../models/asset'); -const JOB_NAME = 'scraper'; const metascraper = require('metascraper'); @@ -12,29 +11,27 @@ const metascraper = require('metascraper'); const scraper = { /** - * creates a new scraper job and scrapes the url when it gets processed. + * Create the new Task kue. + */ + task: new kue.Task({ + name: 'scraper' + }), + + /** + * Creates a new scraper job and scrapes the url when it gets processed. */ create(asset) { - return new Promise((resolve, reject) => { - debug(`Creating job for Asset[${asset.id}]`); - let job = kue.queue - .create(JOB_NAME, { - title: `Scrape for asset ${asset.id}`, - asset_id: asset.id - }) - .attempts(3) - .delay(1000) - .backoff({type: 'exponential'}) - .save((err) => { - if (err) { - return reject(err); - } + debug(`Creating job for Asset[${asset.id}]`); - debug(`Created Job[${job.id}] for Asset[${asset.id}]`); + return scraper.task.create({ + title: `Scrape for asset ${asset.id}`, + asset_id: asset.id + }).then((job) => { - return resolve(job); - }); + debug(`Created Job[${job.id}] for Asset[${asset.id}]`); + + return job; }); }, @@ -48,6 +45,9 @@ const scraper = { })); }, + /** + * Updates an Asset based on scraped asset metadata. + */ update(id, meta) { return Asset.update({id}, { $set: { @@ -68,10 +68,9 @@ const scraper = { */ process() { - debug(`Now processing ${JOB_NAME} jobs`); + debug(`Now processing ${scraper.task.name} jobs`); - // Process jobs with the processJob function. - kue.queue.process(JOB_NAME, (job, done) => { + scraper.task.process((job, done) => { debug(`Starting on Job[${job.id}] for Asset[${job.data.asset_id}]`); @@ -111,27 +110,6 @@ const scraper = { done(err); }); }); - }, - - /** - * Shuts down the current queue to ensure that the application can shutdown - * cleanly. - */ - shutdown() { - return new Promise((resolve, reject) => { - - // Shutdown and give the queue 5 seconds to shutdown before we start - // killing jobs. - kue.queue.shutdown(5000, (err) => { - if (err) { - return reject(err); - } - - debug(`Processing for ${JOB_NAME} jobs stopped`); - - resolve(); - }); - }); } }; diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index dd408d135..ca7b0d558 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -19,22 +19,29 @@ describe('/api/v1/auth', () => { }); }); +const Setting = require('../../../../models/setting'); +const settings = {id: '1'}; + describe('/api/v1/auth/local', () => { - beforeEach(() => { - return User.createLocalUser('maria@gmail.com', 'password!', 'Maria'); - }); + beforeEach(() => Promise.all([ + User.createLocalUser('maria@gmail.com', 'password!', 'Maria'), + Setting.init(settings) + ])); describe('#post', () => { it('should send back the user on a successful login', () => { return chai.request(app) .post('/api/v1/auth/local') .send({email: 'maria@gmail.com', password: 'password!'}) - .catch((res) => { + .then((res) => { expect(res).to.have.status(200); expect(res).to.be.json; expect(res.body).to.have.property('user'); expect(res.body.user).to.have.property('displayName', 'Maria'); + }) + .catch((err) => { + console.error(err); }); }); diff --git a/views/password-reset-email.ejs b/views/email/password-reset.ejs similarity index 58% rename from views/password-reset-email.ejs rename to views/email/password-reset.ejs index 17ed9e39b..e478ceeba 100644 --- a/views/password-reset-email.ejs +++ b/views/email/password-reset.ejs @@ -1,6 +1,2 @@ -

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 %>

-<% } %> diff --git a/views/email/password-reset.txt.ejs b/views/email/password-reset.txt.ejs new file mode 100644 index 000000000..1e44a6629 --- /dev/null +++ b/views/email/password-reset.txt.ejs @@ -0,0 +1,5 @@ +We received a request to reset your password, click here to reset your password: + +<%= rootURL %>/admin/password-reset#<%= token %> + +If you did not request this change, you can ignore this email. diff --git a/views/password-reset.ejs b/views/password-reset.ejs index 1ffd1b554..8ad289dff 100644 --- a/views/password-reset.ejs +++ b/views/password-reset.ejs @@ -117,9 +117,9 @@ } $.ajax({ - url: '/api/v1/users/update-password', + url: '/api/v1/account/password/reset', contentType: 'application/json', - method: 'POST', + method: 'PUT', data: JSON.stringify({password: password, token: location.hash.replace('#', '')}) }).then(function (success) { location.href = '<%= redirectUri %>'; From 298e1e8d7366759321329a7acf58f3a0d89112f2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 4 Jan 2017 10:16:51 -0700 Subject: [PATCH 2/6] Updates to user cli + e2e + tests - Updates to before + beforeEach for mongooose - Removed reference to dotenv from cli in e2e, should use NODE_ENV=test instead. - Changed test port from 30?? to 3000 to be consistent with what nightwatch was expecting --- bin/cli-serve | 2 +- bin/cli-users | 16 +++++--- scripts/pree2e.sh | 6 +-- tests/e2e/tests/Visitor/SignUpTest.js | 5 ++- tests/models/action.js | 42 +++++++++++--------- tests/mongoose.js | 55 +++++++++++++++++++-------- 6 files changed, 80 insertions(+), 46 deletions(-) diff --git a/bin/cli-serve b/bin/cli-serve index 5095c6362..c8cf37a47 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -14,7 +14,7 @@ const util = require('../util'); * Get port from environment and store in Express. */ -const port = normalizePort(process.env.TALK_PORT || (process.env.NODE_ENV === 'test' ? '3011' : '3000')); +const port = normalizePort(process.env.TALK_PORT || '3000'); app.set('port', port); diff --git a/bin/cli-users b/bin/cli-users index ae688e122..fd9e30c7f 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -80,12 +80,16 @@ function createUser(options) { .then((user) => { console.log(`Created user ${user.id}.`); - return User - .addRoleToUser(user.id, result.role.trim()) - .then(() => { - console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); - util.shutdown(); - }); + if (result.role && result.role.length > 0) { + return User + .addRoleToUser(user.id, result.role.trim()) + .then(() => { + console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); + util.shutdown(); + }); + } else { + util.shutdown(); + } }) .catch((err) => { console.error(err); diff --git a/scripts/pree2e.sh b/scripts/pree2e.sh index d8624e81d..0487a9187 100755 --- a/scripts/pree2e.sh +++ b/scripts/pree2e.sh @@ -4,12 +4,12 @@ selenium-standalone install # Creating Admin Test User -{ echo admin@test.com; echo test; echo test; echo Admin Test User; echo admin;} | ./bin/cli-users create +./bin/cli-users create --flag_mode --email "admin@test.com" --password "test" --name "Admin Test User" --role "admin" # Creating Moderator Test User -{ echo moderator@test.com; echo test; echo test; echo Moderator Test User; echo moderator;} | ./bin/cli-users create +./bin/cli-users create --flag_mode --email "moderator@test.com" --password "test" --name "Moderator Test User" --role "moderator" # Creating Commenter Test User -{ echo commenter@test.com; echo test; echo test; echo Commenter Test User; echo ;} | ./bin/cli-users create +./bin/cli-users create --flag_mode --email "commenter@test.com" --password "test" --name "commenter@test.com" npm start & diff --git a/tests/e2e/tests/Visitor/SignUpTest.js b/tests/e2e/tests/Visitor/SignUpTest.js index 4c7650630..9399b47ca 100644 --- a/tests/e2e/tests/Visitor/SignUpTest.js +++ b/tests/e2e/tests/Visitor/SignUpTest.js @@ -1,3 +1,5 @@ +const uuid = require('uuid'); + module.exports = { '@tags': ['signup', 'visitor'], before: client => { @@ -9,11 +11,10 @@ module.exports = { }, 'Visitor signs up': client => { const embedStreamPage = client.page.embedStreamPage(); - const hash = Math.floor(Math.random() * (999 - 0)); embedStreamPage .signUp({ - email: `visitor_${hash}@test.com`, + email: `visitor_${uuid.v4()}@test.com`, displayName: 'Visitor', pass: 'testtest' }); diff --git a/tests/models/action.js b/tests/models/action.js index 5f05dea71..3af55a640 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -4,24 +4,30 @@ const expect = require('chai').expect; describe('models.Action', () => { let mockActions = []; - beforeEach(() => Action.create([{ - action_type: 'flag', - item_id: '123', - item_type: 'comment', - user_id: 'flagginguserid' - }, { - action_type: 'flag', - item_id: '456', - item_type: 'comment' - }, { - action_type: 'flag', - item_id: '123', - item_type: 'comment' - }, { - action_type: 'like', - item_id: '123', - item_type: 'comment' - }]).then((actions) => { + beforeEach(() => Action.create([ + { + action_type: 'flag', + item_id: '123', + item_type: 'comment', + user_id: 'flagginguserid' + }, + { + action_type: 'flag', + item_id: '456', + item_type: 'comment' + }, + { + action_type: 'flag', + item_id: '123', + item_type: 'comment' + }, + { + action_type: 'like', + item_id: '123', + item_type: 'comment' + } + ]).then((actions) => { + console.log('all created'); mockActions = actions; })); diff --git a/tests/mongoose.js b/tests/mongoose.js index 7544072ec..eed246629 100644 --- a/tests/mongoose.js +++ b/tests/mongoose.js @@ -1,27 +1,50 @@ const mongoose = require('../services/mongoose'); -beforeEach(function (done) { - function clearDB() { - for (let collection in mongoose.connection.collections) { - mongoose.connection.collections[collection].remove(function() {}); - } - return done(); - } - - if (mongoose.connection.readyState === 0) { - mongoose.on('open', function() { +function waitTillConnect() { + return new Promise((resolve, reject) => { + mongoose.connection.on('open', function(err) { if (err) { - throw err; + return reject(err); } - return clearDB(); + return resolve(); + }); + }); +} + +before(function(done) { + this.timeout(30000); + + waitTillConnect() + .then(() => { + done(); + }) + .catch((err) => { + done(err); }); - } else { - return clearDB(); - } }); -after(function (done) { +beforeEach(function(done) { + Promise.all(Object.keys(mongoose.connection.collections).map((collection) => { + return new Promise((resolve, reject) => { + mongoose.connection.collections[collection].remove(function(err) { + if (err) { + return reject(err); + } + + return resolve(); + }); + }); + })) + .then(() => { + done(); + }) + .catch((err) => { + done(err); + }); +}); + +after(function(done) { mongoose.disconnect(); return done(); }); From 864b08135a408e3116fd336d7c250e46e2101001 Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 16:24:17 -0300 Subject: [PATCH 3/6] Moves the packages to dependencies. --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 9cedc7a82..800d945e3 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,8 @@ "cli-table": "^0.3.1", "commander": "^2.9.0", "connect-redis": "^3.1.0", + "cookie-parser": "^1.4.3", + "csurf": "^1.9.0", "debug": "^2.2.0", "ejs": "^2.5.2", "env-rewrite": "^1.0.2", @@ -92,9 +94,7 @@ "babel-preset-stage-0": "^6.16.0", "chai": "^3.5.0", "chai-http": "^3.0.0", - "cookie-parser": "^1.4.3", "copy-webpack-plugin": "^4.0.0", - "csurf": "^1.9.0", "css-loader": "^0.25.0", "dialog-polyfill": "^0.4.4", "enzyme": "^2.6.0", From be72ebf2a1c83ffeae2de17ef668d7326fba1b0e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 5 Jan 2017 17:01:41 -0700 Subject: [PATCH 4/6] Added tests + model implementation. --- client/coral-framework/actions/user.js | 2 +- models/setting.js | 3 +- models/user.js | 125 +++++++++++++++++++++---- routes/api/account/index.js | 53 ++++++++--- tests/models/action.js | 1 - tests/models/user.js | 68 ++++++++++++++ tests/routes/api/auth/index.js | 91 ++++++++++++------ 7 files changed, 279 insertions(+), 64 deletions(-) diff --git a/client/coral-framework/actions/user.js b/client/coral-framework/actions/user.js index b4b7b7896..208fc8f5d 100644 --- a/client/coral-framework/actions/user.js +++ b/client/coral-framework/actions/user.js @@ -14,7 +14,7 @@ const saveBioFailure = error => ({type: actions.SAVE_BIO_FAILURE, error}); export const saveBio = (user_id, formData) => dispatch => { dispatch(saveBioRequest()); - coralApi('/account/bio', {method: 'PUT', body: formData}) + coralApi('/account/settings', {method: 'PUT', body: formData}) .then(() => { dispatch(addNotification('success', lang.t('successBioUpdate'))); dispatch(saveBioSuccess(formData)); diff --git a/models/setting.js b/models/setting.js index 9e71e36aa..ad9fa6599 100644 --- a/models/setting.js +++ b/models/setting.js @@ -101,7 +101,8 @@ SettingSchema.method('filterForUser', function(user = false) { 'closeTimeout', 'closedMessage', 'charCountEnable', - 'charCount' + 'charCount', + 'requireEmailConfirmation' ]); } diff --git a/models/user.js b/models/user.js index 0f8fa3e79..5a0e7a640 100644 --- a/models/user.js +++ b/models/user.js @@ -6,6 +6,9 @@ const jwt = require('jsonwebtoken'); const Action = require('./action'); const Comment = require('./comment'); +const EMAIL_CONFIRM_JWT_SUBJECT = 'email_confirm'; +const PASSWORD_RESET_JWT_SUBJECT = 'password_reset'; + // SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run // through during the salting process. const SALT_ROUNDS = 10; @@ -526,27 +529,45 @@ UserService.createPasswordResetToken = function (email) { version: user.__v }; - return jwt.sign(payload, process.env.TALK_SESSION_SECRET, {algorithm: 'HS256'}, { - expiresIn: '1d' + return jwt.sign(payload, process.env.TALK_SESSION_SECRET, { + algorithm: 'HS256', + expiresIn: '1d', + subject: PASSWORD_RESET_JWT_SUBJECT }); }); }; /** - * Verifies a jwt and returns the associated user. - * @param {String} token the JSON Web Token to verify + * Verifies that the token was indeed signed by the session secret. + * @param {String} token JWT token from the client + * @return {Promise} */ -UserService.verifyPasswordResetToken = token => { +UserService.verifyToken = (token, options = {}) => { return new Promise((resolve, reject) => { - jwt.verify(token, process.env.TALK_SESSION_SECRET, (err, decoded) => { + + // Set the allowed algorithms. + options.algorithms = ['HS256']; + + jwt.verify(token, process.env.TALK_SESSION_SECRET, options, (err, decoded) => { if (err) { return reject(err); } resolve(decoded); }); - }) - .then(decoded => UserService.findById(decoded.userId)); + }); +}; + +/** + * Verifies a jwt and returns the associated user. + * @param {String} token the JSON Web Token to verify + */ +UserService.verifyPasswordResetToken = (token) => { + return UserService + .verifyToken(token, { + subject: PASSWORD_RESET_JWT_SUBJECT + }) + .then((decoded) => UserService.findById(decoded.userId)); }; /** @@ -587,27 +608,23 @@ UserService.search = (value) => { * Returns a count of the current users. * @return {Promise} */ -UserService.count = () => { - return UserModel.count(); -}; +UserService.count = () => UserModel.count(); /** * Returns all the users. * @return {Promise} */ -UserService.all = () => { - return UserModel.find(); -}; +UserService.all = () => UserModel.find(); /** - * Adds a new User bio + * Updates the user's settings. * @return {Promise} */ -UserService.addBio = (id, bio) => UserModel.update({ +UserService.updateSettings = (id, settings) => UserModel.update({ id }, { $set: { - 'settings.bio': bio + settings } }); @@ -625,3 +642,77 @@ UserService.addAction = (item_id, user_id, action_type, metadata) => Action.inse action_type, metadata }); + +/** + * This creates a token based around confirming the local profile. + * @param {String} userID The user id for the user that we are creating the + * token for. + * @param {String} email The email that we are needing to get confirmed. + * @return {Promise} + */ +UserService.createEmailConfirmToken = (userID, email) => { + if (!email || typeof email !== 'string') { + return Promise.reject('email is required when creating a JWT for resetting passord'); + } + + email = email.toLowerCase(); + + return UserService + .findById(userID) + .then((user) => { + if (!user) { + return Promise.reject(new Error('user not found')); + } + + // Get the profile representing the local account. + let profile = user.profiles.find((profile) => profile.id === email && profile.provider === 'local'); + + // Ensure that the user email hasn't already been verified. + if (profile && profile.metadata && profile.metadata.confirmed_at) { + return Promise.reject(new Error('email address already confirmed')); + } + + const payload = { + email, + userID + }; + + return jwt.sign(payload, process.env.TALK_SESSION_SECRET, { + jwtid: uuid.v4(), + algorithm: 'HS256', + expiresIn: '1d', + subject: EMAIL_CONFIRM_JWT_SUBJECT + }); + }); +}; + +/** + * This verifies that a given token was for the email confirmation and updates + * that user's profile with a 'confirmed_at' parameter with the current date. + * @param {String} token the token containing the email confirmation details + * signed with our secret. + * @return {Promise} + */ +UserService.verifyEmailConfirmation = (token) => { + return UserService + .verifyToken(token, { + subject: EMAIL_CONFIRM_JWT_SUBJECT + }) + .then(({userID, email}) => { + + return UserModel + .update({ + id: userID, + profiles: { + $elemMatch: { + id: email, + provider: 'local' + } + } + }, { + $set: { + 'profiles.$.metadata.confirmed_at': new Date() + } + }); + }); +}; diff --git a/routes/api/account/index.js b/routes/api/account/index.js index b975ba284..645b9c652 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -4,10 +4,46 @@ const User = require('../../../models/user'); const mailer = require('../../../services/mailer'); const authorization = require('../../../middleware/authorization'); +// ErrPasswordTooShort is returned when the password length is too short. +const ErrPasswordTooShort = new Error('password must be at least 8 characters'); +ErrPasswordTooShort.status = 400; + +// ErrMissingToken is returned in the event that the password reset is requested +// without a token. +const ErrMissingToken = new Error('token is required'); +ErrMissingToken.status = 400; + +//============================================================================== +// ROUTES +//============================================================================== + router.get('/', authorization.needed(), (req, res, next) => { res.json(req.user); }); +// POST /email/confirm takes the password confirmation token available as a +// payload parameter and if it verifies, it updates the confirmed_at date on the +// local profile. +router.post('/email/confirm', (req, res, next) => { + + const { + token + } = req.body; + + if (!token) { + return next(ErrMissingToken); + } + + User + .verifyEmailConfirmation(token) + .then(() => { + res.status(204).end(); + }) + .catch((err) => { + next(err); + }); +}); + /** * 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 @@ -52,15 +88,6 @@ router.post('/password/reset', (req, res, next) => { }); }); -// ErrPasswordTooShort is returned when the password length is too short. -const ErrPasswordTooShort = new Error('password must be at least 8 characters'); -ErrPasswordTooShort.status = 400; - -// ErrMissingToken is returned in the event that the password reset is requested -// without a token. -const ErrMissingToken = new Error('token is required'); -ErrMissingToken.status = 400; - /** * expects 2 fields in the body of the request * 1) the token that was in the url of the email link {String} @@ -95,18 +122,14 @@ router.put('/password/reset', (req, res, next) => { }); }); -router.put('/bio', authorization.needed(), (req, res, next) => { +router.put('/settings', authorization.needed(), (req, res, next) => { const { bio } = req.body; - if (!bio) { - return next(new Error('You must submit a new bio')); - } - User - .addBio(req.user.id, bio) + .updateSettings(req.user.id, {bio}) .then(() => { res.status(204).end(); }) diff --git a/tests/models/action.js b/tests/models/action.js index 3af55a640..a1ebdc027 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -27,7 +27,6 @@ describe('models.Action', () => { item_type: 'comment' } ]).then((actions) => { - console.log('all created'); mockActions = actions; })); diff --git a/tests/models/user.js b/tests/models/user.js index 3883e2e42..7484880bf 100644 --- a/tests/models/user.js +++ b/tests/models/user.js @@ -80,6 +80,74 @@ describe('models.User', () => { }); + describe('#createEmailConfirmToken', () => { + + it('should create a token for a valid user', () => { + return User + .createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id) + .then((token) => { + expect(token).to.not.be.null; + }); + }); + + it('should not create a token for a user already verified', () => { + return User + .createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id) + .then((token) => { + expect(token).to.not.be.null; + + return User.verifyEmailConfirmation(token); + }) + .then(() => { + return User.createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id); + }) + .catch((err) => { + expect(err).to.have.property('message', 'email address already confirmed'); + }); + }); + + }); + + describe('#verifyEmailConfirmation', () => { + + it('should correctly validate a valid token', () => { + return User + .createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id) + .then((token) => { + expect(token).to.not.be.null; + + return User.verifyEmailConfirmation(token); + }); + }); + + it('should correctly reject an invalid token', () => { + return User + .verifyEmailConfirmation('cats') + .catch((err) => { + expect(err).to.not.be.null; + }); + }); + + it('should update the user model when verification is complete', () => { + return User + .createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id) + .then((token) => { + expect(token).to.not.be.null; + + return User.verifyEmailConfirmation(token); + }) + .then(() => { + return User.findById(mockUsers[0].id); + }) + .then((user) => { + expect(user.profiles[0]).to.have.property('metadata'); + expect(user.profiles[0].metadata).to.have.property('confirmed_at'); + expect(user.profiles[0].metadata.confirmed_at).to.not.be.null; + }); + }); + + }); + describe('#setStatus', () => { it('should set the status to active', () => { return User diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index ca7b0d558..4848a94c7 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -20,40 +20,73 @@ describe('/api/v1/auth', () => { }); const Setting = require('../../../../models/setting'); -const settings = {id: '1'}; describe('/api/v1/auth/local', () => { - beforeEach(() => Promise.all([ - User.createLocalUser('maria@gmail.com', 'password!', 'Maria'), - Setting.init(settings) - ])); + let mockUser; + beforeEach(() => User.createLocalUser('maria@gmail.com', 'password!', 'Maria').then((user) => { + mockUser = user; + })); - describe('#post', () => { - it('should send back the user on a successful login', () => { - return chai.request(app) - .post('/api/v1/auth/local') - .send({email: 'maria@gmail.com', password: 'password!'}) - .then((res) => { - expect(res).to.have.status(200); - expect(res).to.be.json; - expect(res.body).to.have.property('user'); - expect(res.body.user).to.have.property('displayName', 'Maria'); - }) - .catch((err) => { - console.error(err); - }); - }); + describe('email confirmation disabled', () => { + + beforeEach(() => Setting.init({requireEmailConfirmation: false})); + + describe('#post', () => { + it('should send back the user on a successful login', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!'}) + .then((res) => { + expect(res).to.have.status(200); + expect(res).to.be.json; + expect(res.body).to.have.property('user'); + expect(res.body.user).to.have.property('displayName', 'Maria'); + }); + }); + + it('should not send back the user on a unsuccessful login', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!3'}) + .catch((err) => { + expect(err).to.not.be.null; + expect(err.response).to.have.status(401); + expect(err.response.body).to.have.property('message', 'not authorized'); + }); + }); - it('should not send back the user on a unsuccessful login', () => { - return chai.request(app) - .post('/api/v1/auth/local') - .send({email: 'maria@gmail.com', password: 'password!3'}) - .catch((err) => { - expect(err).to.not.be.null; - expect(err.response).to.have.status(401); - expect(err.response.body).to.have.property('message', 'not authorized'); - }); }); }); + + describe('email confirmation enabled', () => { + + beforeEach(() => Setting.init({requireEmailConfirmation: true})); + + describe('#post', () => { + it('should not allow a login from a user that is not confirmed', () => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!'}) + .catch((err) => { + err.response.should.have.status(401); + + return User.createEmailConfirmToken(mockUser.id, mockUser.profiles[0].id); + }) + .then(User.verifyEmailConfirmation) + .then(() => { + return chai.request(app) + .post('/api/v1/auth/local') + .send({email: 'maria@gmail.com', password: 'password!'}); + }) + .then((res) => { + expect(res).to.have.status(200); + expect(res).to.be.json; + expect(res.body).to.have.property('user'); + expect(res.body.user).to.have.property('displayName', 'Maria'); + }); + }); + }); + + }); }); From cabe546ecf393c9afda6008dbb5f357b4be6203c Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 5 Jan 2017 17:19:38 -0700 Subject: [PATCH 5/6] Added email send on new user if enabled for confirming --- routes/api/users/index.js | 41 +++++++++++++++++++++++++++++-- views/email/email-confirm.ejs | 3 +++ views/email/email-confirm.txt.ejs | 9 +++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 views/email/email-confirm.ejs create mode 100644 views/email/email-confirm.txt.ejs diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 8dc48d5dc..8f1bf7d41 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -1,6 +1,8 @@ const express = require('express'); const router = express.Router(); const User = require('../../../models/user'); +const Setting = require('../../../models/setting'); +const mailer = require('../../../services/mailer'); const authorization = require('../../../middleware/authorization'); router.get('/', authorization.needed('admin'), (req, res, next) => { @@ -59,8 +61,43 @@ router.post('/', authorization.needed('admin'), (req, res, next) => { User .createLocalUser(email, password, displayName) - .then(user => { - res.status(201).json(user); + .then((user) => { + + // Get the settings from the database to find out if we need to send an + // email confirmation. The Front end will know about the + // requireEmailConfirmation as it's included in the settings get endpoint. + return Setting.retrieve().then(({requireEmailConfirmation = false}) => { + + if (requireEmailConfirmation) { + + // Email confirmation is required, let's generate that token and send + // the email. + return User + .createEmailConfirmToken(user.id, email) + .then((token) => { + return mailer.sendSimple({ + app: req.app, // needed to render the templates. + template: 'email/email-confirm', // needed to know which template to render! + locals: { // specifies the template locals. + token, + rootURL: process.env.TALK_ROOT_URL, + email + }, + subject: 'Email Confirmation - Talk', + to: email + }); + }) + .then(() => { + + // Then send back the user. + res.status(201).json(user); + }); + } else { + + // We don't need to confirm the email, let's just send back the user! + res.status(201).json(user); + } + }); }) .catch(err => { next(err); diff --git a/views/email/email-confirm.ejs b/views/email/email-confirm.ejs new file mode 100644 index 000000000..da1b0e815 --- /dev/null +++ b/views/email/email-confirm.ejs @@ -0,0 +1,3 @@ +

A email confirmation has been requested for the following account: <%= email %>.

+

To confirm the account, please visit the following link: http://example.com/email/confirm/endpoint#<%= token %>

+

If you did not request this, you can safely ignore this email.

diff --git a/views/email/email-confirm.txt.ejs b/views/email/email-confirm.txt.ejs new file mode 100644 index 000000000..764991d8d --- /dev/null +++ b/views/email/email-confirm.txt.ejs @@ -0,0 +1,9 @@ +A email confirmation has been requested for the following account: + + <%= email %> + +To confirm the account, please visit the following link: + + http://example.com/email/confirm/endpoint#<%= token %> + +If you did not request this, you can safely ignore this email. From 4c5b85a2aeb67f7c8a3c423ffb874fb46344b411 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 5 Jan 2017 19:02:06 -0700 Subject: [PATCH 6/6] Fixed session bug --- app.js | 1 + routes/api/auth/index.js | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app.js b/app.js index c9785cf76..aca80a4b5 100644 --- a/app.js +++ b/app.js @@ -43,6 +43,7 @@ const session_opts = { rolling: true, saveUninitialized: false, resave: false, + unset: 'destroy', name: 'talk.sid', cookie: { secure: false, diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index ae508b679..c7cfde9b5 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -24,9 +24,9 @@ router.get('/', (req, res, next) => { * This destroys the session of a user, if they have one. */ router.delete('/', authorization.needed(), (req, res) => { - req.session.destroy(() => { - res.status(204).end(); - }); + delete req.session.passport; + + res.status(204).end(); }); //==============================================================================