From 20878ba040225bb05989948d0438895ff66fe0b7 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 21 Dec 2017 23:27:37 -0700 Subject: [PATCH 1/5] css and standardization on all admin pages --- .eslintignore | 2 +- .nodemon.json | 2 +- config.js | 6 +- errors.js | 17 +- locales/en.yml | 11 + middleware/staticTemplate.js | 21 +- .../client/components/ApproveCommentAction.js | 2 +- public/css/admin.css | 71 +++++++ public/javascripts/admin.js | 8 + routes/admin/index.js | 6 - routes/api/account/index.js | 60 +++--- routes/api/users/index.js | 4 +- routes/embed/index.js | 12 +- routes/index.js | 4 +- services/email/email-confirm.txt.ejs | 2 +- services/i18n.js | 11 + services/mailer.js | 188 +++++++++--------- services/users.js | 40 +++- test/server/routes/api/auth/index.js | 2 +- test/server/services/users.js | 8 +- views/admin.ejs | 7 +- views/admin/confirm-email.ejs | 84 +++----- views/admin/docs.ejs | 5 +- views/admin/password-reset.ejs | 117 ++--------- views/embed/stream.ejs | 2 +- 25 files changed, 367 insertions(+), 325 deletions(-) create mode 100644 public/css/admin.css create mode 100644 public/javascripts/admin.js 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 7f7fd3d59..9077398bc 100644 --- a/.nodemon.json +++ b/.nodemon.json @@ -1,5 +1,5 @@ { "verbose": true, "ignore": ["test/*", "client/*", "dist/*", "plugins/*/client"], - "ext": "js,json,graphql" + "ext": "js,json,graphql,yml" } diff --git a/config.js b/config.js index 9637611fc..ab4622581 100644 --- a/config.js +++ b/config.js @@ -175,11 +175,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 3d145d197..cfabf5ec2 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 }); @@ -231,7 +239,8 @@ module.exports = { ErrMaxRateLimit, ErrMissingEmail, ErrMissingPassword, - ErrMissingToken, + ErrEmailVerificationToken, + ErrPasswordResetToken, ErrMissingUsername, ErrNotAuthorized, ErrNotFound, @@ -244,4 +253,4 @@ module.exports = { ErrSpecialChars, ErrUsernameTaken, ExtendableError, -}; \ No newline at end of file +}; diff --git a/locales/en.yml b/locales/en.yml index 9257ecdcf..796b7b9a1 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -14,6 +14,15 @@ en: yes_ban_user: "Yes, Ban User" 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" @@ -184,6 +193,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..7f6184a03 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,22 @@ 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; diff --git a/plugins/talk-plugin-moderation-actions/client/components/ApproveCommentAction.js b/plugins/talk-plugin-moderation-actions/client/components/ApproveCommentAction.js index 125a3589e..0f16ca595 100644 --- a/plugins/talk-plugin-moderation-actions/client/components/ApproveCommentAction.js +++ b/plugins/talk-plugin-moderation-actions/client/components/ApproveCommentAction.js @@ -26,4 +26,4 @@ ApproveCommentAction.propTypes = { comment: PropTypes.object, }; -export default ApproveCommentAction; \ No newline at end of file +export default ApproveCommentAction; 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 c35f709cb..3cc4f6f87 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -17,27 +17,31 @@ router.get('/', authorization.needed(), (req, res, next) => { // 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; + const {token, check} = req.body; if (!token) { - return next(errors.ErrMissingToken); + return next(errors.ErrEmailVerificationToken); + } + + if (check) { + try { + await UsersService.verifyEmailConfirmationToken(token); + return res.status(204).end(); + } catch (err) { + console.error(err); + return next(errors.ErrEmailVerificationToken); + } } 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 +52,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 +68,34 @@ 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; - const {token, password} = req.body; + const {token, password, check = false} = req.body; if (!token) { - return next(errors.ErrMissingToken); + return next(errors.ErrPasswordResetToken); } - if (check !== 'true' && (!password || password.length < 8)) { + if (check) { + try { + await UsersService.verifyPasswordResetToken(token); + return res.status(204).end(); + } catch (err) { + console.error(err); + return next(errors.ErrPasswordResetToken); + } + } + + 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/api/users/index.js b/routes/api/users/index.js index 3199c25ab..056276a0e 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -99,7 +99,7 @@ router.post('/:user_id/email', authorization.needed('ADMIN', 'MODERATOR'), async return next(errors.ErrMissingEmail); } - await mailer.sendSimple({ + await mailer.send({ template: 'notification', // needed to know which template to render! locals: { // specifies the template locals. body: req.body.body @@ -122,7 +122,7 @@ router.post('/:user_id/email', authorization.needed('ADMIN', 'MODERATOR'), async const SendEmailConfirmation = async (user, email, referer) => { let token = await UsersService.createEmailConfirmToken(user, email, referer); - return mailer.sendSimple({ + return mailer.send({ template: 'email-confirm', locals: { token, 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 af7edec3b..c563eb434 100644 --- a/routes/index.js +++ b/routes/index.js @@ -173,7 +173,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 { @@ -189,7 +189,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(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/i18n.js b/services/i18n.js index cd1bacd93..14597e89b 100644 --- a/services/i18n.js +++ b/services/i18n.js @@ -32,6 +32,14 @@ let translations = fs.readdirSync(resolve()) // Create a list of all supported translations. const languages = Object.keys(translations); +// Move the default language to the front. +if (languages.includes(DEFAULT_LANG)) { + const from = languages.indexOf(DEFAULT_LANG); + languages.splice(from, 1); + languages.splice(0, 0, DEFAULT_LANG); +} +debug(`loaded language sets for ${languages}`); + let loadedPluginTranslations = false; const loadPluginTranslations = () => { if (loadedPluginTranslations) { @@ -80,8 +88,11 @@ const t = (language) => (key, ...replacements) => { */ const i18n = { request(req) { + debug(`possible languages given request '${accepts(req).languages()}'`); const lang = accepts(req).language(languages); + debug(`parsed request language as '${lang}'`); const language = lang ? lang : DEFAULT_LANG; + debug(`decided language as '${language}'`); return t(language); }, diff --git a/services/mailer.js b/services/mailer.js index 4433b9ef5..b276a719f 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 {attachStaticLocals} = require('../middleware/staticTemplate'); const i18n = require('./i18n'); @@ -54,102 +54,108 @@ 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_HOST.length > 0 && + SMTP_USERNAME && SMTP_USERNAME.length > 0 && + SMTP_PORT && SMTP_PORT.length > 0 && + SMTP_PASSWORD && SMTP_PASSWORD.length > 0 && + SMTP_FROM_ADDRESS && SMTP_FROM_ADDRESS.length > 0 +) || 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 ({template, locals, to, subject}) => { + if (!mailer.enabled) { + throw new Error('email is not enabled because required configuration is not available'); } + // Attach the template locals. + attachStaticLocals(locals); + + // Attach the translation function. + locals.t = i18n.t; + + // Render the templates. + const [ + html, + text, + ] = await Promise.all(['html', 'txt'].map((fmt) => { + return templates.render(template, fmt, locals); + })); + + // Create the job. + return mailer.task.create({ + title: 'Mail', + message: { + to, + subject: `${EMAIL_SUBJECT_PREFIX} ${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 ef3cf7a34..e61714d9f 100644 --- a/services/users.js +++ b/services/users.js @@ -439,7 +439,7 @@ module.exports = class UsersService { subject: i18n.t('email.banned.subject'), to: localProfile.id }; - await MailerService.sendSimple(options); + await MailerService.send(options); } } @@ -475,7 +475,7 @@ module.exports = class UsersService { to: localProfile.id, }; - await MailerService.sendSimple(options); + await MailerService.send(options); } } @@ -511,7 +511,7 @@ module.exports = class UsersService { // We may want a standard way to access a user's e-mail address in the future }; - await MailerService.sendSimple(options); + await MailerService.send(options); } } @@ -767,6 +767,36 @@ module.exports = class UsersService { }, tokenOptions); } + static async verifyEmailConfirmationToken(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. @@ -775,9 +805,7 @@ module.exports = 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/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 b4a15e3ba..0293800eb 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()', () => { @@ -160,7 +160,7 @@ describe('services.UsersService', () => { expect(user).to.have.property('status', 'ACTIVE'); }) .then(() => { - expect(MailerService.sendSimple).to.not.have.been.called; + expect(MailerService.send).to.not.have.been.called; }); }); @@ -203,7 +203,7 @@ describe('services.UsersService', () => { expect(user).to.have.property('status', 'BANNED'); }) .then(() => { - expect(MailerService.sendSimple).to.have.been.calledWithMatch({ + expect(MailerService.send).to.have.been.calledWithMatch({ template: 'banned', to: mockUsers[0].profiles[0].id }); diff --git a/views/admin.ejs b/views/admin.ejs index 70fed054e..9cb8b8ad3 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 ae2fe2dcf..f66040be4 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') %> - +
+ From 82c84de33fdfc14cfe683c299d66b9135a7417dd Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 3 Jan 2018 14:22:52 -0700 Subject: [PATCH 2/5] code review --- middleware/staticTemplate.js | 1 + routes/index.js | 2 +- services/mailer.js | 28 +++++++++++++--------------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/middleware/staticTemplate.js b/middleware/staticTemplate.js index 7f6184a03..c94797519 100644 --- a/middleware/staticTemplate.js +++ b/middleware/staticTemplate.js @@ -54,3 +54,4 @@ module.exports = async (req, res, next) => { }; module.exports.attachStaticLocals = attachStaticLocals; +module.exports.TEMPLATE_LOCALS = TEMPLATE_LOCALS; diff --git a/routes/index.js b/routes/index.js index c563eb434..74e37999a 100644 --- a/routes/index.js +++ b/routes/index.js @@ -189,7 +189,7 @@ router.use('/', (err, req, res, next) => { if (err instanceof errors.APIError) { res.status(err.status); res.render('error', { - message: res.locals.t(err.translation_key), + message: res.locals.t(`error.${err.translation_key}`), error: process.env.NODE_ENV === 'development' ? err : {} }); } else { diff --git a/services/mailer.js b/services/mailer.js index b276a719f..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 {attachStaticLocals} = require('../middleware/staticTemplate'); +const {TEMPLATE_LOCALS} = require('../middleware/staticTemplate'); const i18n = require('./i18n'); @@ -62,11 +62,11 @@ const mailer = {}; // 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_HOST.length > 0 && - SMTP_USERNAME && SMTP_USERNAME.length > 0 && - SMTP_PORT && SMTP_PORT.length > 0 && - SMTP_PASSWORD && SMTP_PASSWORD.length > 0 && - SMTP_FROM_ADDRESS && SMTP_FROM_ADDRESS.length > 0 + SMTP_HOST && + SMTP_USERNAME && + SMTP_PORT && + SMTP_PASSWORD && + SMTP_FROM_ADDRESS ) || process.env.NODE_ENV === 'test'; if (mailer.enabled) { @@ -101,31 +101,29 @@ mailer.task = new kue.Task({ /** * send will create a new message and send it. */ -mailer.send = async ({template, locals, to, subject}) => { +mailer.send = async (options) => { if (!mailer.enabled) { throw new Error('email is not enabled because required configuration is not available'); } - // Attach the template locals. - attachStaticLocals(locals); - - // Attach the translation function. - locals.t = i18n.t; + // 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(template, fmt, locals); + return templates.render(options.template, fmt, locals); })); // Create the job. return mailer.task.create({ title: 'Mail', message: { - to, - subject: `${EMAIL_SUBJECT_PREFIX} ${subject}`, + to: options.to, + subject: `${EMAIL_SUBJECT_PREFIX} ${options.subject}`, text, html } From 375fbd37909ccb61d43adc7606b452114914ca2e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 3 Jan 2018 14:32:19 -0700 Subject: [PATCH 3/5] resolving code climate issues --- routes/api/account/index.js | 54 ++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/routes/api/account/index.js b/routes/api/account/index.js index 3cc4f6f87..b58fd2ce0 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -13,26 +13,44 @@ 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/verify', async (req, res, next) => { - const {token, check} = req.body; +/** + * 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 verifyTokenOnCheck = (verifier, error) => async (req, res, next) => { + const {token, check = false} = req.body; if (!token) { - return next(errors.ErrEmailVerificationToken); + return next(error); } if (check) { try { - await UsersService.verifyEmailConfirmationToken(token); - return res.status(204).end(); + await verifier(token); + + 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; } catch (err) { console.error(err); - return next(errors.ErrEmailVerificationToken); + return next(error); } } + 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', verifyTokenOnCheck(UsersService.verifyEmailConfirmationToken, errors.ErrEmailVerificationToken), async (req, res, next) => { + const {token} = req.body; + try { let {referer} = await UsersService.verifyEmailConfirmation(token); return res.json({redirectUri: referer}); @@ -68,22 +86,8 @@ router.post('/password/reset', async (req, res, next) => { } }); -router.put('/password/reset', async (req, res, next) => { - const {token, password, check = false} = req.body; - - if (!token) { - return next(errors.ErrPasswordResetToken); - } - - if (check) { - try { - await UsersService.verifyPasswordResetToken(token); - return res.status(204).end(); - } catch (err) { - console.error(err); - return next(errors.ErrPasswordResetToken); - } - } +router.put('/password/reset', verifyTokenOnCheck(UsersService.verifyPasswordResetToken, errors.ErrPasswordResetToken), async (req, res, next) => { + const {token, password} = req.body; if (!password || password.length < 8) { return next(errors.ErrPasswordTooShort); From 0850b1f98237ec0090207ac6ca30789de38c7829 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 3 Jan 2018 14:34:50 -0700 Subject: [PATCH 4/5] added some comments --- services/users.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/services/users.js b/services/users.js index e61714d9f..a82aee8c4 100644 --- a/services/users.js +++ b/services/users.js @@ -628,7 +628,9 @@ module.exports = 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) { @@ -648,6 +650,7 @@ module.exports = class UsersService { /** * Finds a user using a value which gets compared using a prefix match against * the user's email address and/or their username. + * * @param {String} value value to search by * @return {Promise} */ @@ -767,6 +770,12 @@ module.exports = class UsersService { }, tokenOptions); } + /** + * 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) { const decoded = await UsersService.verifyToken(token, { subject: EMAIL_CONFIRM_JWT_SUBJECT From 72b2211daab79288941d9cd2dfaac4a531af6dca Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 3 Jan 2018 14:43:35 -0700 Subject: [PATCH 5/5] reduced complexity --- routes/api/account/index.js | 31 +++++++++++++++++-------------- services/users.js | 8 ++++++++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/routes/api/account/index.js b/routes/api/account/index.js index b58fd2ce0..434edc58d 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -20,26 +20,29 @@ router.get('/', authorization.needed(), (req, res, next) => { * @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 verifyTokenOnCheck = (verifier, error) => async (req, res, next) => { - const {token, check = false} = req.body; - - if (!token) { - return next(error); - } +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); - - 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; } 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(); @@ -48,7 +51,7 @@ const verifyTokenOnCheck = (verifier, error) => async (req, res, 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', verifyTokenOnCheck(UsersService.verifyEmailConfirmationToken, errors.ErrEmailVerificationToken), async (req, res, next) => { +router.post('/email/verify', tokenCheck(UsersService.verifyEmailConfirmationToken, errors.ErrEmailVerificationToken), async (req, res, next) => { const {token} = req.body; try { @@ -86,7 +89,7 @@ router.post('/password/reset', async (req, res, next) => { } }); -router.put('/password/reset', verifyTokenOnCheck(UsersService.verifyPasswordResetToken, errors.ErrPasswordResetToken), async (req, res, next) => { +router.put('/password/reset', tokenCheck(UsersService.verifyPasswordResetToken, errors.ErrPasswordResetToken), async (req, res, next) => { const {token, password} = req.body; if (!password || password.length < 8) { diff --git a/services/users.js b/services/users.js index a82aee8c4..9159ef5a6 100644 --- a/services/users.js +++ b/services/users.js @@ -634,6 +634,10 @@ module.exports = class UsersService { * @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 }); @@ -777,6 +781,10 @@ module.exports = class UsersService { * @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 });