diff --git a/.eslintignore b/.eslintignore index fd85e4523..7ee579147 100644 --- a/.eslintignore +++ b/.eslintignore @@ -28,5 +28,5 @@ plugins/* !plugins/talk-plugin-deep-reply-count !plugins/talk-plugin-subscriber !plugins/talk-plugin-flag-details - +public node_modules diff --git a/.nodemon.json b/.nodemon.json index 36fed8700..44b17e9ee 100644 --- a/.nodemon.json +++ b/.nodemon.json @@ -2,7 +2,7 @@ "exec": "npm-run-all --parallel generate-introspection start:development", "verbose": true, "ignore": ["test/*", "client/*", "dist/*", "plugins/*/client"], - "ext": "js,json,graphql", + "ext": "js,json,graphql,yml", "watch": [ ".", "bin/cli", diff --git a/config.js b/config.js index 72c24c70f..0d6643269 100644 --- a/config.js +++ b/config.js @@ -207,13 +207,11 @@ const CONFIG = { // SMTP Server configuration //------------------------------------------------------------------------------ - SMTP_FROM_ADDRESS: process.env.TALK_SMTP_FROM_ADDRESS, SMTP_HOST: process.env.TALK_SMTP_HOST, - SMTP_PASSWORD: process.env.TALK_SMTP_PASSWORD, - SMTP_PORT: process.env.TALK_SMTP_PORT - ? parseInt(process.env.TALK_SMTP_PORT) - : undefined, SMTP_USERNAME: process.env.TALK_SMTP_USERNAME, + SMTP_PORT: process.env.TALK_SMTP_PORT, + SMTP_PASSWORD: process.env.TALK_SMTP_PASSWORD, + SMTP_FROM_ADDRESS: process.env.TALK_SMTP_FROM_ADDRESS, //------------------------------------------------------------------------------ // Flagging Config diff --git a/errors.js b/errors.js index a87d234e8..fb4b0341d 100644 --- a/errors.js +++ b/errors.js @@ -69,9 +69,17 @@ const ErrMissingUsername = new APIError('A username is required to create a user status: 400 }); -// ErrMissingToken is returned in the event that the password reset is requested +// ErrEmailVerificationToken is returned in the event that the password reset is requested // without a token. -const ErrMissingToken = new APIError('token is required', { +const ErrEmailVerificationToken = new APIError('token is required', { + translation_key: 'EMAIL_VERIFICATION_TOKEN_INVALID', + status: 400 +}); + +// ErrPasswordResetToken is returned in the event that the password reset is requested +// without a token. +const ErrPasswordResetToken = new APIError('token is required', { + translation_key: 'PASSWORD_RESET_TOKEN_INVALID', status: 400 }); @@ -225,7 +233,8 @@ module.exports = { ErrMaxRateLimit, ErrMissingEmail, ErrMissingPassword, - ErrMissingToken, + ErrEmailVerificationToken, + ErrPasswordResetToken, ErrMissingUsername, ErrNotAuthorized, ErrNotFound, diff --git a/locales/en.yml b/locales/en.yml index 2926d140a..a4238d5c0 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -19,6 +19,15 @@ en: email_message_ban: "Dear {0},\n\nSomeone with access to your account has violated our community guidelines. As a result, your account has been banned. You will no longer be able to comment, like or report comments. if you think this has been done in error, please contact our community team." bio_offensive: "This bio is offensive" cancel: "Cancel" + confirm_email: + click_to_confirm: "Click below to confirm your email address" + confirm: "Confirm" + password_reset: + set_new_password: "Change Your Password" + new_password: "New Password" + new_password_help: "Password must be at least 8 characters" + confirm_new_password: "Confirm New Password" + change_password: "Change Password" characters_remaining: "characters remaining" comment: anon: "Anonymous" @@ -189,6 +198,8 @@ en: embedlink: copy: "Copy to Clipboard" error: + EMAIL_VERIFICATION_TOKEN_INVALID: "Email verification token is invalid." + PASSWORD_RESET_TOKEN_INVALID: "Your password reset link is invalid." COMMENT_TOO_SHORT: "Comments should be more than one character, please revise your comment and try again." NOT_AUTHORIZED: "You are not authorized to perform this action." NO_SPECIAL_CHARACTERS: "Usernames can contain letters numbers and _ only" diff --git a/middleware/staticTemplate.js b/middleware/staticTemplate.js index 823ab6624..c94797519 100644 --- a/middleware/staticTemplate.js +++ b/middleware/staticTemplate.js @@ -1,3 +1,5 @@ +const SettingsService = require('../services/settings'); + const { BASE_URL, BASE_PATH, @@ -24,8 +26,8 @@ const TEMPLATE_LOCALS = { }, }; -// attachLocals will attach the locals to the response only. -const attachLocals = (locals) => { +// attachStaticLocals will attach the locals to the response only. +const attachStaticLocals = (locals) => { for (const key in TEMPLATE_LOCALS) { const value = TEMPLATE_LOCALS[key]; @@ -33,13 +35,23 @@ const attachLocals = (locals) => { } }; -module.exports = (req, res, next) => { +module.exports = async (req, res, next) => { + + try { + + // Attach the custom css url. + const {customCssUrl} = await SettingsService.retrieve('customCssUrl'); + res.locals.customCssUrl = customCssUrl; + } catch (err) { + console.warn(err); + } // Always attach the locals. - attachLocals(res.locals); + attachStaticLocals(res.locals); // Forward the request. next(); }; -module.exports.attachLocals = attachLocals; +module.exports.attachStaticLocals = attachStaticLocals; +module.exports.TEMPLATE_LOCALS = TEMPLATE_LOCALS; diff --git a/public/css/admin.css b/public/css/admin.css new file mode 100644 index 000000000..50e3109c5 --- /dev/null +++ b/public/css/admin.css @@ -0,0 +1,71 @@ +body, #root { + width: 100%; + height: 100%; + margin: 0; + background: #fff; +} + +.container { + max-width: 300px; + margin: 50px auto; +} + +#root form { + display: none; + padding: 15px; +} + +.legend { + text-align: center; + width: 100%; + font-weight: bold; +} + +label { + display: block; + margin-top: 10px; + margin-bottom: 3px; + padding-right: 30px; +} + +small { + color: #888; +} + +input { + border-radius: 4px; + margin-top: 3px; + border: 1px solid lightgrey; + font-size: 16px; + width: 100%; + padding: 14px; + height: 100%; + display: inline-block; +} + +button[type="submit"] { + border-radius: 4px; + border: none; + display: block; + background-color: #333; + color: white; + text-align: center; + width: 100%; + padding: 10px; + margin-top: 10px; + cursor: pointer; +} + +.error-console { + display: none; + margin-top: 10px; + border-radius: 4px; + background-color: pink; + color: red; + border: 1px solid red; + padding: 10px; +} + +.error-console.active { + display: block; +} \ No newline at end of file diff --git a/public/javascripts/admin.js b/public/javascripts/admin.js new file mode 100644 index 000000000..2c8b7c42a --- /dev/null +++ b/public/javascripts/admin.js @@ -0,0 +1,8 @@ +function showError(error) { + try { + let err = JSON.parse(error); + $('.error-console').text(err.message).addClass('active'); + } catch (err) { + $('.error-console').text(error).addClass('active'); + } +} diff --git a/routes/admin/index.js b/routes/admin/index.js index d6cb481de..66f9c123d 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,17 +1,11 @@ const express = require('express'); const router = express.Router(); -// Get /email-confirmation expects a signed JWT in the hash router.get('/confirm-email', (req, res) => { res.render('admin/confirm-email'); }); -// Get /password-reset expects a signed token (JWT) in the hash. -// Links to this endpoint are generated by /views/password-reset-email.ejs. router.get('/password-reset', (req, res) => { - - // TODO: store the redirect uri in the token or something fancy. - // admins and regular users should probably be redirected to different places. res.render('admin/password-reset'); }); diff --git a/routes/api/account/index.js b/routes/api/account/index.js index de3555725..f134ec0e5 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -13,31 +13,56 @@ router.get('/', authorization.needed(), (req, res, next) => { res.json(req.user); }); +/** + * verifyTokenOnCheck will verify that the request contains a token, and if + * being checked, will return the check status to the user. + * + * @param {Function} verifier the function used to verify the token, will throw on error + * @param {Object} error the error object to send back in the event an error is found + */ +const tokenCheck = (verifier, error) => async (req, res, next) => { + const {token = null, check = false} = req.body; + + if (check) { + + // This request is checking to see if the token is valid. + try { + + // Verify the token. + await verifier(token); + } catch (err) { + + // Log out the error, slurp it and send out the predefined error to the + // error handler. + console.error(err); + return next(error); + } + + res.status(204).end(); + + // Don't continue to pass it onto the next middleware, as we've only been + // asked to verify the token. + return; + } + + next(); +}; + // 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/verify', async (req, res, next) => { - - const { - token - } = req.body; - - if (!token) { - return next(errors.ErrMissingToken); - } +router.post('/email/verify', tokenCheck(UsersService.verifyEmailConfirmationToken, errors.ErrEmailVerificationToken), async (req, res, next) => { + const {token} = req.body; try { let {referer} = await UsersService.verifyEmailConfirmation(token); - res.json({redirectUri: referer}); - } catch (e) { - return next(e); + return res.json({redirectUri: referer}); + } catch (err) { + console.error(err); + return next(errors.ErrEmailVerificationToken); } }); -/** - * 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', async (req, res, next) => { const {email, loc} = req.body; @@ -48,7 +73,7 @@ router.post('/password/reset', async (req, res, next) => { try { let token = await UsersService.createPasswordResetToken(email, loc); if (token) { - await mailer.sendSimple({ + await mailer.send({ template: 'password-reset', locals: { token, @@ -64,34 +89,20 @@ router.post('/password/reset', async (req, res, 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.put('/password/reset', async (req, res, next) => { - const {check} = req.query; +router.put('/password/reset', tokenCheck(UsersService.verifyPasswordResetToken, errors.ErrPasswordResetToken), async (req, res, next) => { const {token, password} = req.body; - if (!token) { - return next(errors.ErrMissingToken); - } - - if (check !== 'true' && (!password || password.length < 8)) { + if (!password || password.length < 8) { return next(errors.ErrPasswordTooShort); } try { - let [user, loc] = await UsersService.verifyPasswordResetToken(token); - if (check === 'true') { - res.status(204).end(); - return; - } + let [user, redirect] = await UsersService.verifyPasswordResetToken(token); // Change the users' password. await UsersService.changePassword(user.id, password); - res.json({redirect: loc}); + res.json({redirect}); } catch (e) { console.error(e); return next(errors.ErrNotAuthorized); diff --git a/routes/embed/index.js b/routes/embed/index.js index 852d9afa7..695ffe609 100644 --- a/routes/embed/index.js +++ b/routes/embed/index.js @@ -1,16 +1,8 @@ const express = require('express'); const router = express.Router(); -const SettingsService = require('../../services/settings'); -router.use('/:embed', async (req, res, next) => { - switch (req.params.embed) { - case 'stream': { - const {customCssUrl} = await SettingsService.retrieve('customCssUrl'); - return res.render('embed/stream', {customCssUrl}); - } - } - - return next(); +router.use('/stream', (req, res) => { + res.render('embed/stream'); }); module.exports = router; diff --git a/routes/index.js b/routes/index.js index 506ed3cfb..3f7041654 100644 --- a/routes/index.js +++ b/routes/index.js @@ -174,7 +174,7 @@ router.use('/api', (err, req, res, next) => { if (err instanceof errors.APIError) { res.status(err.status).json({ - message: err.message, + message: res.locals.t(`error.${err.translation_key}`), error: err }); } else { @@ -190,7 +190,7 @@ router.use('/', (err, req, res, next) => { if (err instanceof errors.APIError) { res.status(err.status); res.render('error', { - message: err.message, + message: res.locals.t(`error.${err.translation_key}`), error: process.env.NODE_ENV === 'development' ? err : {} }); } else { diff --git a/services/email/email-confirm.txt.ejs b/services/email/email-confirm.txt.ejs index d327220a7..b3cf28a01 100644 --- a/services/email/email-confirm.txt.ejs +++ b/services/email/email-confirm.txt.ejs @@ -4,6 +4,6 @@ <%= t('email.confirm.to_confirm') %> - <%= BASE_URL %>confirm/endpoint#<%= token %> + <%= BASE_URL %>admin/confirm-email#<%= token %> <%= t('email.confirm.if_you_did_not') %> diff --git a/services/mailer.js b/services/mailer.js index 4433b9ef5..ecb65de8c 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -4,7 +4,7 @@ const kue = require('./kue'); const path = require('path'); const fs = require('fs'); const _ = require('lodash'); -const {attachLocals} = require('../middleware/staticTemplate'); +const {TEMPLATE_LOCALS} = require('../middleware/staticTemplate'); const i18n = require('./i18n'); @@ -54,102 +54,106 @@ templates.render = (name, format = 'txt', context) => new Promise((resolve, reje return resolve(view(context)); }); -}); // ends templates.render +}); -const options = { - host: SMTP_HOST, - auth: { - user: SMTP_USERNAME, - pass: SMTP_PASSWORD - } -}; +const mailer = {}; -if (SMTP_PORT) { - try { - options.port = parseInt(SMTP_PORT); - } catch (e) { - throw new Error('TALK_SMTP_PORT is not an integer'); +// enabled is true when the required configuration is available. When testing +// is enabled, we will be simulating that emails are being sent, because in a +// production system, emails should and would be sent. +mailer.enabled = Boolean( + SMTP_HOST && + SMTP_USERNAME && + SMTP_PORT && + SMTP_PASSWORD && + SMTP_FROM_ADDRESS +) || process.env.NODE_ENV === 'test'; + +if (mailer.enabled) { + const options = { + host: SMTP_HOST, + auth: { + user: SMTP_USERNAME, + pass: SMTP_PASSWORD + } + }; + + if (SMTP_PORT) { + try { + options.port = parseInt(SMTP_PORT); + } catch (e) { + throw new Error('TALK_SMTP_PORT is not an integer'); + } + } else { + options.port = 25; } -} else { - options.port = 25; + + mailer.transport = nodemailer.createTransport(options); } -const defaultTransporter = nodemailer.createTransport(options); +/** + * Create the new Task kue. + */ +mailer.task = new kue.Task({ + name: 'mailer' +}); -const mailer = module.exports = { - - /** - * Create the new Task kue. - */ - task: new kue.Task({ - name: 'mailer' - }), - - sendSimple({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'); - } - - // Prefix the subject with `[Talk]`. - subject = `${EMAIL_SUBJECT_PREFIX} ${subject}`; - - attachLocals(locals); - - // Attach the translation function. - locals.t = i18n.t; - - return Promise.all([ - - // Render the HTML version of the email. - templates.render(template, 'html', locals), - - // Render the TEXT version of the email. - templates.render(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 = 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(); - }); - }); +/** + * send will create a new message and send it. + */ +mailer.send = async (options) => { + if (!mailer.enabled) { + throw new Error('email is not enabled because required configuration is not available'); } + // Create the new locals object and attach the static locals and the i18n + // framework. + const locals = _.merge({}, options.locals, TEMPLATE_LOCALS, {t: i18n.t}); + + // Render the templates. + const [ + html, + text, + ] = await Promise.all(['html', 'txt'].map((fmt) => { + return templates.render(options.template, fmt, locals); + })); + + // Create the job. + return mailer.task.create({ + title: 'Mail', + message: { + to: options.to, + subject: `${EMAIL_SUBJECT_PREFIX} ${options.subject}`, + text, + html + } + }); }; + +/** + * Start the queue processor for the mailer job. + */ +mailer.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 = SMTP_FROM_ADDRESS; + + // Actually send the email. + mailer.transport.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/users.js b/services/users.js index c0fc47d9b..face22f9f 100644 --- a/services/users.js +++ b/services/users.js @@ -452,7 +452,7 @@ class UsersService { redirectURI ); - return MailerService.sendSimple({ + return MailerService.send({ template: 'email-confirm', locals: { token, @@ -478,7 +478,7 @@ class UsersService { to, }); - return MailerService.sendSimple(options); + return MailerService.send(options); } static async changePassword(id, password) { @@ -741,10 +741,16 @@ class UsersService { } /** - * Verifies a jwt and returns the associated user. + * Verifies a jwt and returns the associated user. Throws an error when the + * token isn't valid. + * * @param {String} token the JSON Web Token to verify */ static async verifyPasswordResetToken(token) { + if (!token) { + throw new Error('cannot verify an empty token'); + } + const {userId, loc, version} = await UsersService.verifyToken(token, { subject: PASSWORD_RESET_JWT_SUBJECT, }); @@ -851,6 +857,46 @@ class UsersService { ); } + /** + * verifyEmailConfirmationToken checks the validity of a given token without + * actually confirming the user's email address. + * + * @param {String} token the token to verify + */ + static async verifyEmailConfirmationToken(token) { + if (!token) { + throw new Error('cannot verify an empty token'); + } + + const decoded = await UsersService.verifyToken(token, { + subject: EMAIL_CONFIRM_JWT_SUBJECT + }); + + const user = await UserModel.findOne({ + id: decoded.userID, + profiles: { + $elemMatch: { + id: decoded.email, + provider: 'local', + }, + }, + }); + if (!user) { + throw errors.ErrNotFound; + } + + const profile = user.profiles.find(({id}) => id === decoded.email); + if (!profile) { + throw errors.ErrNotFound; + } + + if (profile.metadata && profile.metadata.confirmed_at !== null) { + throw errors.ErrEmailVerificationToken; + } + + return decoded; + } + /** * 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. @@ -859,9 +905,7 @@ class UsersService { * @return {Promise} */ static async verifyEmailConfirmation(token) { - let {userID, email, referer} = await UsersService.verifyToken(token, { - subject: EMAIL_CONFIRM_JWT_SUBJECT, - }); + let {userID, email, referer} = await UsersService.verifyEmailConfirmationToken(token); await UsersService.confirmEmail(userID, email); diff --git a/test/server/graph/mutations/setUserBanStatus.js b/test/server/graph/mutations/setUserBanStatus.js index 6dc1c3167..00efaa0c5 100644 --- a/test/server/graph/mutations/setUserBanStatus.js +++ b/test/server/graph/mutations/setUserBanStatus.js @@ -22,7 +22,7 @@ describe('graph.mutations.banUser', () => { let spy; before(() => { - spy = sinon.spy(MailerService, 'sendSimple'); + spy = sinon.spy(MailerService, 'send'); }); afterEach(() => { diff --git a/test/server/graph/mutations/setUserSuspensionStatus.js b/test/server/graph/mutations/setUserSuspensionStatus.js index 79604d7c4..d636951d6 100644 --- a/test/server/graph/mutations/setUserSuspensionStatus.js +++ b/test/server/graph/mutations/setUserSuspensionStatus.js @@ -24,7 +24,7 @@ describe('graph.mutations.suspendUser', () => { let spy; before(() => { - spy = sinon.spy(MailerService, 'sendSimple'); + spy = sinon.spy(MailerService, 'send'); }); afterEach(() => { diff --git a/test/server/routes/api/auth/index.js b/test/server/routes/api/auth/index.js index 463e27711..8e3061938 100644 --- a/test/server/routes/api/auth/index.js +++ b/test/server/routes/api/auth/index.js @@ -54,7 +54,7 @@ describe('/api/v1/auth/local', () => { .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'); + expect(err.response.body).to.have.property('message', 'You are not authorized to perform this action.'); }); }); diff --git a/test/server/services/users.js b/test/server/services/users.js index dffbc0ee6..8be3fc89b 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -29,11 +29,11 @@ describe('services.UsersService', () => { password: '3Coral!3' }]); - sinon.spy(MailerService, 'sendSimple'); + sinon.spy(MailerService, 'send'); }); afterEach(() => { - MailerService.sendSimple.restore(); + MailerService.send.restore(); }); describe('#findById()', () => { diff --git a/views/admin.ejs b/views/admin.ejs index 9bbee7e32..da689f07f 100644 --- a/views/admin.ejs +++ b/views/admin.ejs @@ -34,12 +34,15 @@ height: 100%; } + <%_ if (locals.customCssUrl) { _%> + + <%_ } _%> <% if (data != null) { %> - + <% } %> - +
diff --git a/views/admin/confirm-email.ejs b/views/admin/confirm-email.ejs index ed23d3dc8..0bc54990b 100644 --- a/views/admin/confirm-email.ejs +++ b/views/admin/confirm-email.ejs @@ -6,68 +6,25 @@ Email Verification - - + + <%_ if (locals.customCssUrl) { _%> + + <%_ } _%> - +
-
-
-

Verify Email Address

-
-
- Click the button below to verify the email on your new user account. -
-
- - Verify - - -
-
- +
+
+ <%= t('confirm_email.click_to_confirm') %> + +
- - + diff --git a/views/admin/docs.ejs b/views/admin/docs.ejs index cf1dc0b3c..7c6ad9251 100644 --- a/views/admin/docs.ejs +++ b/views/admin/docs.ejs @@ -25,8 +25,11 @@ font-weight: bold; } + <%_ if (locals.customCssUrl) { _%> + + <%_ } _%> - +
diff --git a/views/admin/password-reset.ejs b/views/admin/password-reset.ejs index 6725409d1..d5163f2fd 100644 --- a/views/admin/password-reset.ejs +++ b/views/admin/password-reset.ejs @@ -6,111 +6,33 @@ Password Reset - - + + <%_ if (locals.customCssUrl) { _%> + + <%_ } _%> - +
- Set new password + <%= t('password_reset.set_new_password') %> - +
+