From 20878ba040225bb05989948d0438895ff66fe0b7 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 21 Dec 2017 23:27:37 -0700 Subject: [PATCH 01/26] 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 02/26] 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 03/26] 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 04/26] 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 05/26] 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 }); From 4a259d09e4872b2721c7ea6d991b0cc67e8b22f6 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 5 Jan 2018 18:56:13 +0100 Subject: [PATCH 06/26] Fix styling for new x button --- client/coral-ui/components/Drawer.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-ui/components/Drawer.css b/client/coral-ui/components/Drawer.css index 88c30425f..1e3055184 100644 --- a/client/coral-ui/components/Drawer.css +++ b/client/coral-ui/components/Drawer.css @@ -23,6 +23,7 @@ } .closeButton { + composes: buttonReset from 'coral-framework/styles/reset.css'; position: absolute; width: 40px; height: 40px; @@ -34,7 +35,6 @@ top: 60px; box-shadow: -1px 3px 4px 0px rgba(0,0,0,0.15); text-align: center; - padding-top: 10px; cursor: pointer; &:hover { From 49c350376b20c54a5ddeb1dc9ccd9476ed106aab Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 5 Jan 2018 18:56:44 +0100 Subject: [PATCH 07/26] Use null instead of boolean --- client/coral-admin/src/components/UserDetail.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/coral-admin/src/components/UserDetail.js b/client/coral-admin/src/components/UserDetail.js index b75793ab7..bdac0e246 100644 --- a/client/coral-admin/src/components/UserDetail.js +++ b/client/coral-admin/src/components/UserDetail.js @@ -128,9 +128,9 @@ class UserDetail extends React.Component { const banned = isBanned(user); const suspended = isSuspended(user); - + return ( - +

{user.username} From 7538e6e8fd5d5b91460a9387e9532d971cc4f35f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 5 Jan 2018 18:57:12 +0100 Subject: [PATCH 08/26] Remove unused state --- .../src/routes/Moderation/containers/Comment.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/containers/Comment.js b/client/coral-admin/src/routes/Moderation/containers/Comment.js index 35f11a3fb..72b0c23ab 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Comment.js +++ b/client/coral-admin/src/routes/Moderation/containers/Comment.js @@ -37,19 +37,6 @@ export default withFragments({ user { id username - state { - status { - username { - status - } - banned { - status - } - suspension { - until - } - } - } } asset { id From c01b192a736c96a3daafcfa4db94850142d6b5eb Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 5 Jan 2018 19:02:35 +0100 Subject: [PATCH 09/26] Mark onClickOutside not required --- client/coral-framework/components/ClickOutside.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/coral-framework/components/ClickOutside.js b/client/coral-framework/components/ClickOutside.js index 054a9e6fe..ea60247ca 100644 --- a/client/coral-framework/components/ClickOutside.js +++ b/client/coral-framework/components/ClickOutside.js @@ -4,7 +4,8 @@ import {findDOMNode} from 'react-dom'; export default class ClickOutside extends Component { static propTypes = { - onClickOutside: PropTypes.func.isRequired + onClickOutside: PropTypes.func, + children: PropTypes.node, }; static contextTypes = { @@ -16,7 +17,7 @@ export default class ClickOutside extends Component { handleClick = (e) => { const {onClickOutside} = this.props; if (!e || !this.domNode.contains(e.target)) { - onClickOutside(e); + onClickOutside && onClickOutside(e); } }; From acdaaffd4ee551cb8671fae443011a058f39274f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 5 Jan 2018 19:03:40 +0100 Subject: [PATCH 10/26] Add proptype --- client/coral-admin/src/components/UserDetail.js | 1 + 1 file changed, 1 insertion(+) diff --git a/client/coral-admin/src/components/UserDetail.js b/client/coral-admin/src/components/UserDetail.js index bdac0e246..4b3d4e9bc 100644 --- a/client/coral-admin/src/components/UserDetail.js +++ b/client/coral-admin/src/components/UserDetail.js @@ -314,6 +314,7 @@ UserDetail.propTypes = { showBanUserDialog: PropTypes.func, unbanUser: PropTypes.func.isRequired, unsuspendUser: PropTypes.func.isRequired, + modal: PropTypes.bool, }; export default UserDetail; From b42cce57475f04d76733b565b727b8bd780971e1 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 11:12:35 -0700 Subject: [PATCH 11/26] resolving pr concerns --- graph/loaders/users.js | 5 ++--- plugins/talk-plugin-featured-comments/index.js | 4 ++-- services/actions.js | 5 ++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/graph/loaders/users.js b/graph/loaders/users.js index 679d3dea5..112fe21bc 100644 --- a/graph/loaders/users.js +++ b/graph/loaders/users.js @@ -1,7 +1,6 @@ const DataLoader = require('dataloader'); const util = require('./util'); -const sc = require('snake-case'); const { SEARCH_OTHER_USERS, @@ -121,7 +120,7 @@ const getUsersByQuery = async ({user}, {limit, cursor, value = '', state, action if (action_type) { query.merge({ - [`action_counts.${sc(action_type.toLowerCase())}`]: { + [`action_counts.${action_type.toLowerCase()}`]: { $gt: 0 } }); @@ -199,7 +198,7 @@ const getCountByQuery = async ({user}, {action_type, state}) => { if (action_type) { query.merge({ - [`action_counts.${sc(action_type.toLowerCase())}`]: { + [`action_counts.${action_type.toLowerCase()}`]: { $gt: 0 } }); diff --git a/plugins/talk-plugin-featured-comments/index.js b/plugins/talk-plugin-featured-comments/index.js index c81afa3ea..2d68bac50 100644 --- a/plugins/talk-plugin-featured-comments/index.js +++ b/plugins/talk-plugin-featured-comments/index.js @@ -57,7 +57,7 @@ module.exports = { hooks: { RootMutation: { addTag: { - async post(obj, {tag: {name, id, item_type}}, {user, mutators: {Comment}, pubsub}, _info) { + async post(obj, {tag: {name, id, item_type}}, {user, mutators: {Comment}, pubsub}) { if (name === 'FEATURED' && item_type === 'COMMENTS') { const comment = await Comment.setStatus({id: id, status: 'ACCEPTED'}); if (comment) { @@ -67,7 +67,7 @@ module.exports = { }, }, removeTag: { - async post(obj, {tag: {name, id, item_type}}, {user, loaders: {Comments}, pubsub}, _info) { + async post(obj, {tag: {name, id, item_type}}, {user, loaders: {Comments}, pubsub}) { if (name === 'FEATURED' && item_type === 'COMMENTS') { const comment = await Comments.get.load(id); if (comment) { diff --git a/services/actions.js b/services/actions.js index adffdd134..cc6f9afa9 100644 --- a/services/actions.js +++ b/services/actions.js @@ -1,7 +1,6 @@ const ActionModel = require('../models/action'); const CommentModel = require('../models/comment'); const UserModel = require('../models/user'); -const sc = require('snake-case'); const _ = require('lodash'); const errors = require('../errors'); const events = require('./events'); @@ -253,14 +252,14 @@ module.exports = class ActionsService { }; const incrActionCounts = async (action, value) => { - const ACTION_TYPE = sc(action.action_type.toLowerCase()); + const ACTION_TYPE = action.action_type.toLowerCase(); const update = { [`action_counts.${ACTION_TYPE}`]: value, }; if (action.group_id && action.group_id.length > 0) { - const GROUP_ID = sc(action.group_id.toLowerCase()); + const GROUP_ID = action.group_id.toLowerCase(); update[`action_counts.${ACTION_TYPE}_${GROUP_ID}`] = value; } From 7b5993bc606c7959289317e58555939f2b6a27f3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 11:17:04 -0700 Subject: [PATCH 12/26] fixed wrong query logic --- services/users.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/users.js b/services/users.js index face22f9f..0727aca53 100644 --- a/services/users.js +++ b/services/users.js @@ -140,7 +140,7 @@ class UsersService { let user = await UserModel.findOneAndUpdate( { id, - status: { + 'status.banned.status': { $ne: status, }, }, @@ -189,7 +189,7 @@ class UsersService { let user = await UserModel.findOneAndUpdate( { id, - status: { + 'status.username.status': { $ne: status, }, }, From c9bc9a4b10681738fd6939bbccb370a064b2fdc3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 11:31:40 -0700 Subject: [PATCH 13/26] reintroduced the errors.ErrSameUsernameProvided --- errors.js | 6 ++++++ services/users.js | 2 +- test/server/services/users.js | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/errors.js b/errors.js index fb4b0341d..9078dc866 100644 --- a/errors.js +++ b/errors.js @@ -59,6 +59,11 @@ const ErrUsernameTaken = new APIError('Username already in use', { status: 400 }); +const ErrSameUsernameProvided = new APIError('Username provided for change is the same as current', { + translation_key: 'SAME_USERNAME_PROVIDED', + status: 400 +}); + const ErrSpecialChars = new APIError('No special characters are allowed in a username', { translation_key: 'NO_SPECIAL_CHARACTERS', status: 400 @@ -245,5 +250,6 @@ module.exports = { ErrSettingsNotInit, ErrSpecialChars, ErrUsernameTaken, + ErrSameUsernameProvided, ExtendableError, }; diff --git a/services/users.js b/services/users.js index 0727aca53..f2dfee166 100644 --- a/services/users.js +++ b/services/users.js @@ -281,7 +281,7 @@ class UsersService { } if (!resetAllowed && user.username === username) { - throw errors.ErrUsernameTaken; + throw errors.ErrSameUsernameProvided; } throw new Error('edit username failed for an unexpected reason'); diff --git a/test/server/services/users.js b/test/server/services/users.js index 8be3fc89b..343b73bdf 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -238,7 +238,7 @@ describe('services.UsersService', () => { await UsersService[func](user.id, user.username); throw new Error('edit was processed successfully'); } catch (err) { - expect(err).have.property('translation_key', 'USERNAME_IN_USE'); + expect(err).have.property('translation_key', 'SAME_USERNAME_PROVIDED'); } } else { await UsersService[func](user.id, user.username); From c3970a46a2939be94f7d47e71203fd0d822fefca Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 12:11:32 -0700 Subject: [PATCH 14/26] applied rename to graph mutation edges --- client/coral-framework/graphql/mutations.js | 4 ++-- graph/resolvers/root_mutation.js | 4 ++-- graph/typeDefs.graphql | 4 ++-- test/server/graph/mutations/setUserBanStatus.js | 4 ++-- test/server/graph/mutations/setUserSuspensionStatus.js | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/client/coral-framework/graphql/mutations.js b/client/coral-framework/graphql/mutations.js index 89db451ae..0086d9a00 100644 --- a/client/coral-framework/graphql/mutations.js +++ b/client/coral-framework/graphql/mutations.js @@ -180,7 +180,7 @@ export const withSuspendUser = withMutation( export const withUnsuspendUser = withMutation( gql` mutation UnSuspendUser($input: UnSuspendUserInput!) { - unSuspendUser(input: $input) { + unsuspendUser(input: $input) { ...UnSuspendUserResponse } } @@ -276,7 +276,7 @@ export const withBanUser = withMutation( export const withUnbanUser = withMutation( gql` mutation UnBanUser($input: UnBanUserInput!) { - unBanUser(input: $input) { + unbanUser(input: $input) { ...UnBanUserResponse } } diff --git a/graph/resolvers/root_mutation.js b/graph/resolvers/root_mutation.js index ac64bdad7..c1e484860 100644 --- a/graph/resolvers/root_mutation.js +++ b/graph/resolvers/root_mutation.js @@ -34,13 +34,13 @@ const RootMutation = { suspendUser: async (obj, {input: {id, until, message}}, {mutators: {User}}) => { await User.setUserSuspensionStatus(id, until, message); }, - unSuspendUser: async (obj, {input: {id}}, {mutators: {User}}) => { + unsuspendUser: async (obj, {input: {id}}, {mutators: {User}}) => { await User.setUserSuspensionStatus(id); }, banUser: async (obj, {input: {id, message}}, {mutators: {User}}) => { await User.setUserBanStatus(id, true, message); }, - unBanUser: async (obj, {input: {id}}, {mutators: {User}}) => { + unbanUser: async (obj, {input: {id}}, {mutators: {User}}) => { await User.setUserBanStatus(id, false); }, ignoreUser: async (_, {id}, {mutators: {User}}) => { diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 7d39125e0..e39238d4e 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -1396,7 +1396,7 @@ type RootMutation { # Sets the suspension status on a given user. Requires the `MODERATOR` role. # Mutation is restricted. - unSuspendUser(input: UnSuspendUserInput!): UnSuspendUserResponse + unsuspendUser(input: UnSuspendUserInput!): UnSuspendUserResponse # Sets the ban status on a given user. Requires the `MODERATOR` role. # Mutation is restricted. @@ -1404,7 +1404,7 @@ type RootMutation { # Sets the ban status on a given user. Requires the `MODERATOR` role. # Mutation is restricted. - unBanUser(input: UnBanUserInput!): UnBanUserResponse + unbanUser(input: UnBanUserInput!): UnBanUserResponse # Sets the username status on a given user to `APPROVED`. Requires the # `MODERATOR` role. Mutation is restricted. diff --git a/test/server/graph/mutations/setUserBanStatus.js b/test/server/graph/mutations/setUserBanStatus.js index 00efaa0c5..37a6bac62 100644 --- a/test/server/graph/mutations/setUserBanStatus.js +++ b/test/server/graph/mutations/setUserBanStatus.js @@ -46,7 +46,7 @@ describe('graph.mutations.banUser', () => { } mutation UnBanUser($user_id: ID!) { - unBanUser(input: { + unbanUser(input: { id: $user_id }) { errors { @@ -112,7 +112,7 @@ describe('graph.mutations.banUser', () => { console.error(res.errors); } expect(res.errors).to.be.undefined; - expect(res.data.unBanUser).to.be.null; + expect(res.data.unbanUser).to.be.null; user = await UserModel.findOne({id: user.id}); diff --git a/test/server/graph/mutations/setUserSuspensionStatus.js b/test/server/graph/mutations/setUserSuspensionStatus.js index d636951d6..5ac46fe84 100644 --- a/test/server/graph/mutations/setUserSuspensionStatus.js +++ b/test/server/graph/mutations/setUserSuspensionStatus.js @@ -49,7 +49,7 @@ describe('graph.mutations.suspendUser', () => { } mutation UnSuspendUser($user_id: ID!) { - unSuspendUser(input: { + unsuspendUser(input: { id: $user_id, }) { errors { @@ -124,7 +124,7 @@ describe('graph.mutations.suspendUser', () => { console.error(res.errors); } expect(res.errors).to.be.undefined; - expect(res.data.unSuspendUser).to.be.null; + expect(res.data.unsuspendUser).to.be.null; user = await UserModel.findOne({id: user.id}); From 10754a65c683980d4bea4a4b45ce90bc525eefca Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 12:12:54 -0700 Subject: [PATCH 15/26] don't apply configuration from dotfiles during testing --- config.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/config.js b/config.js index 0d6643269..e617b93e8 100644 --- a/config.js +++ b/config.js @@ -7,9 +7,12 @@ // entrypoint for the entire applications configuration. require('env-rewrite').rewrite(); -// Apply all the configuration provided in the .env file if it isn't already -// in the environment. -require('dotenv').config(); +if (process.env.NODE_ENV !== 'test') { + + // Apply all the configuration provided in the .env file if it isn't already + // in the environment. + require('dotenv').config(); +} const uniq = require('lodash/uniq'); const ms = require('ms'); From 62603395089af8859c9c3b89837811270b0beebe Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 15:32:36 -0700 Subject: [PATCH 16/26] user cli patches - removed unused user actions - added system side graph context - added user cli search --- bin/cli-users | 452 +++++++++++++++------------------------- bin/util.js | 7 + graph/context.js | 12 ++ models/user.js | 19 +- package.json | 1 + perms/reducers/index.js | 7 + services/users.js | 29 --- yarn.lock | 35 +++- 8 files changed, 247 insertions(+), 315 deletions(-) diff --git a/bin/cli-users b/bin/cli-users index c15572fe4..b80af1fab 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -4,124 +4,36 @@ * Module dependencies. */ +const util = require('./util'); const program = require('commander'); const inquirer = require('inquirer'); +const {graphql} = require('graphql'); +const {stripIndent} = require('common-tags'); +const Table = require('cli-table'); + +// Make things colorful! +require('colors'); + +// Register the autocomplete plugin. +inquirer.registerPrompt('autocomplete', require('inquirer-autocomplete-prompt')); + +const schema = require('../graph/schema'); +const Context = require('../graph/context'); const UsersService = require('../services/users'); const UserModel = require('../models/user'); const CommentModel = require('../models/comment'); const ActionModel = require('../models/action'); const USER_ROLES = require('../models/enum/user_roles'); const mongoose = require('../services/mongoose'); -const util = require('./util'); -const Table = require('cli-table'); const databaseVerifications = require('./verifications/database'); -const validateRequired = (msg = 'Field is required', len = 1) => (input) => { - if (input && input.length >= len) { - return true; - } - - return msg; -}; - -// Regeister the shutdown criteria. +// Register the shutdown criteria. util.onshutdown([ () => mongoose.disconnect() ]); -function getUserCreateAnswers(options) { - if (options.flag_mode) { - - let user = { - email: options.email, - password: options.password, - confirmPassword: options.password, - username: options.name, - role: 'COMMENTER' - }; - - if (options.role && USER_ROLES.indexOf(options.role) > -1) { - user.roles = options.role; - } - - return Promise.resolve(user); - } - - return inquirer.prompt([ - { - name: 'email', - message: 'Email', - format: 'email', - validate: validateRequired('Email is required') - }, - { - name: 'password', - message: 'Password', - type: 'password', - filter: (password) => { - return UsersService - .isValidPassword(password) - .catch((err) => { - throw err.message; - }); - } - }, - { - name: 'confirmPassword', - message: 'Confirm Password', - type: 'password', - filter: (confirmPassword) => { - return UsersService - .isValidPassword(confirmPassword) - .catch((err) => { - throw err.message; - }); - } - }, - { - name: 'username', - message: 'Username', - filter: (username) => { - return UsersService - .isValidUsername(username) - .catch((err) => { - throw err.message; - }); - } - }, - { - name: 'role', - message: 'User Role', - type: 'list', - choices: USER_ROLES - } - ]); -} - /** - * Prompts for input and registers a user based on those. - */ -async function createUser(options) { - try { - const answers = await getUserCreateAnswers(options); - if (answers.password !== answers.confirmPassword) { - throw new Error('Passwords do not match'); - } - - const user = await UsersService.createLocalUser(answers.email.trim(), answers.password.trim(), answers.username.trim()); - await UsersService.setRole(user.id, answers.role); - await UsersService.sendEmailConfirmation(user, answers.email.trim()); - - console.log(`Created User ${user.id}.`); - util.shutdown(); - } catch (err) { - console.error(err); - util.shutdown(1); - } -} - -/** - * Deletes a user. + * Deletes a user and cleans up their associated verifications. */ async function deleteUser(userID) { @@ -133,23 +45,50 @@ async function deleteUser(userID) { throw new Error(`user with id ${userID} not found`); } + printUserAsTable(user); + + console.warn(stripIndent` + + This will delete the above user. + + This might take a long time if there is a lot of data, please confirm that + you want to continue. + `); + const {confirm} = await inquirer.prompt({ + type: 'confirm', + name: 'confirm', + message: 'Continue', + default: false, + }); + if (!confirm) { + return util.shutdown(); + } + + console.warn('Removing user\'s actions'); + // Remove all the user's actions. await ActionModel .where({user_id: user.id}) .setOptions({multi: true}) .remove(); + console.warn('Removing user\'s comments'); + // Remove all the user's comments. await CommentModel .where({author_id: user.id}) .setOptions({multi: true}) .remove(); + console.warn('Updating the database indexes'); + // Update the counts that might have changed. for (const verification of databaseVerifications) { await verification({fix: true, limit: Infinity, batch: 1000}); } + console.warn('Removing the user'); + // Remove the user. await user.remove(); @@ -160,146 +99,86 @@ async function deleteUser(userID) { } } +function printUserAsTable(user) { + let table = new Table({}); + + table.push( + {'ID': user.id.gray}, + {'Username': user.username}, + {'Emails': user.profiles.filter(({provider}) => provider === 'local').map(({id})=> id) + .join(', ')}, + {'Tags': user.tags ? user.tags.map(({tag: {name}}) => name) : ''}, + {'Role': user.role}, + {'Verified': user.hasVerifiedEmail}, + {'Username': user.status.username.status}, + {'Banned': user.banned}, + {'Suspension': user.suspended ? `Until ${user.status.suspension.until}` : false}, + ); + + console.log(table.toString()); +} + /** - * Changes the password for a user. + * Searches for users based on their username and email address. */ -function passwd(userID) { - inquirer.prompt([ - { - name: 'password', - message: 'Password', - type: 'password', - validate: validateRequired('Password is required') - }, - { - name: 'confirmPassword', - message: 'Confirm Password', - type: 'password', - validate: validateRequired('Confirm Password is required') +async function searchUsers() { + const ctx = Context.forSystem(); + const searchQuery = ` + query SearchUsers($value: String) { + users(query: {value: $value}) { + nodes { + id + username + role + profiles { + id + provider + } + } + } } - ]) - .then((answers) => { - if (answers.password !== answers.confirmPassword) { - throw new Error('Password mismatch'); - } + `; - return UsersService.changePassword(userID, answers.password); - }) - .then(() => { - console.log('Password changed.'); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); - }); -} - -/** - * Updates the user from the options array. - */ -function updateUser(userID, options) { - const updates = []; - - if (options.email && typeof options.email === 'string' && options.email.length > 0) { - let q = UserModel.update({ - 'id': userID, - 'profiles.provider': 'local' - }, { - $set: { - 'profiles.$.id': options.email - } - }); - - updates.push(q); - } - - if (options.name && typeof options.name === 'string' && options.name.length > 0) { - let q = UserModel.update({ - 'id': userID - }, { - $set: { - username: options.name - } - }); - - updates.push(q); - } - - Promise - .all(updates.map((q) => q.exec())) - .then(() => { - console.log(`User ${userID} updated.`); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); - }); -} - -/** - * Lists all the users registered in the database. - */ -function listUsers() { - UsersService - .all() - .then((users) => { - let table = new Table({ - head: [ - 'ID', - 'Username', - 'Profiles', - 'Roles', - 'Status', - 'State' - ] - }); - - users.forEach((user) => { - const profile = user.profiles.find(({provider}) => provider === 'local'); - let state; - if (profile && profile.metadata && profile.metadata.confirmed_at) { - state = 'Verified'; - } else { - state = 'Unverified'; + try { + const answers = await inquirer.prompt({ + type: 'autocomplete', + name: 'userID', + message: 'Search for a user', + source: async (answers, value) => { + if (value === null) { + value = ''; } - table.push([ - user.id, - user.username, - user.profiles.map((p) => p.provider).join(', '), - user.role, - user.status, - state - ]); - }); + const {data, errors} = await graphql(schema, searchQuery, {}, ctx, { + value, + }); + if (errors && errors.length > 0) { + throw errors[0]; + } - console.log(table.toString()); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); - }); -} + return data.users.nodes.map((user) => { + const emails = user.profiles + .filter(({provider}) => provider === 'local') + .map(({id})=> id) + .join(', '); -/** - * Merges two users using the specified ID's. - * @param {String} dstUserID id of the user to which is the target of the merge - * @param {String} srcUserID id of the user to which is the source of the merge - */ -function mergeUsers(dstUserID, srcUserID) { - UsersService - .mergeUsers(dstUserID, srcUserID) - .then(() => { - console.log(`User ${srcUserID} was merged into user ${dstUserID}.`); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); + return { + name: `${user.username} (${emails}) ${user.id.gray} - ${user.role.gray}`, + value: user.id, + }; + }); + } }); + + const {userID} = answers; + const user = await UserModel.findOne({id: userID}); + + printUserAsTable(user); + util.shutdown(0); + } catch (err) { + console.error(err); + util.shutdown(1); + } } /** @@ -307,20 +186,16 @@ function mergeUsers(dstUserID, srcUserID) { * @param {String} userUD id of the user to add the role to * @param {String} role the role to add */ -async function setRole(userID, role, options) { +async function setUserRole(userID) { try { - if (options.interactive || !role) { - const answers = await inquirer.prompt([ - { - name: 'role', - message: 'User Role', - type: 'list', - choices: USER_ROLES - } - ]); - - role = answers.role; - } + const {role} = await inquirer.prompt([ + { + name: 'role', + message: 'User Role', + type: 'list', + choices: USER_ROLES + } + ]); await UsersService.setRole(userID, role); @@ -337,10 +212,49 @@ async function setRole(userID, role, options) { * Verifies an email address for a user. * * @param userID the user's id - * @param email the user's email address to be verified + * @param email the user's email address to be verified, otherwise verifies the + * first email if there is one, if there are multiple, you get a + * prompt. */ -async function verify(userID, email) { +async function verifyUserEmail(userID, email) { try { + + // Get the user. + const user = await UserModel.findOne({id: userID}); + if (!user) { + throw new Error(`user with ID ${userID} cannot be found`); + } + + // Get all the user's email addresses. + const emails = user.profiles.filter(({provider}) => provider === 'local').map(({id}) => id); + if (!emails || emails.length === 0) { + throw new Error('user did not have any email addresses'); + } + + if (!email && emails.length === 1) { + + // The email wasn't passed, and there is only one option. + email = emails[0]; + } else if (!emails.includes(email)){ + + // The email passed doesn't belong to this user. + throw new Error(`user does not have the email ${email}`); + } else if (emails.length > 1) { + + // The email wasn't passed, and there is more than one choice. + const answers = await inquirer.prompt([ + { + name: 'email', + message: 'Select Email to Verify', + type: 'list', + choices: emails + } + ]); + + email = answers.email; + } + + // Verify the email. await UsersService.confirmEmail(userID, email); console.log(`User ${userID} had their email ${email} verified.`); util.shutdown(); @@ -354,53 +268,25 @@ async function verify(userID, email) { // Setting up the program command line arguments. //============================================================================== -program - .command('create') - .option('--email [email]', 'Email to use') - .option('--password [password]', 'Password to use') - .option('--name [name]', 'Name to use') - .option('--role [role]', 'Role to add') - .option('-f, --flag_mode', 'Source from flags instead of prompting') - .description('create a new user') - .action(createUser); - program .command('delete ') .description('delete a user') .action(deleteUser); program - .command('passwd ') - .description('change a password for a user') - .action(passwd); + .command('search') + .description('searches for a user based on their stored username and email') + .action(searchUsers); program - .command('update ') - .option('--email [email]', 'Email to use') - .option('--name [name]', 'Name to use') - .description('update a user') - .action(updateUser); - -program - .command('list') - .description('list all the users in the database') - .action(listUsers); - -program - .command('merge ') - .description('merge srcUser into the dstUser') - .action(mergeUsers); - -program - .command('setrole [role]') - .option('-i, --interactive', 'Enable interactive mode') + .command('set-role ') .description('sets the role on a user') - .action(setRole); + .action(setUserRole); program .command('verify ') .description('verifies the given user\'s email address') - .action(verify); + .action(verifyUserEmail); program.parse(process.argv); diff --git a/bin/util.js b/bin/util.js index 572d07a2e..abcbeaade 100644 --- a/bin/util.js +++ b/bin/util.js @@ -54,3 +54,10 @@ util.onshutdown = (jobs) => { process.on('SIGTERM', () => util.shutdown(0, 'SIGTERM')); process.on('SIGINT', () => util.shutdown(0, 'SIGINT')); process.once('SIGUSR2', () => util.shutdown(0, 'SIGUSR2')); + +// Makes the script crash on unhandled rejections instead of silently +// ignoring them. In the future, promise rejections that are not handled will +// terminate the Node.js process with a non-zero exit code. +process.on('unhandledRejection', (err) => { + throw err; +}); diff --git a/graph/context.js b/graph/context.js index e894c1534..bc4e782ca 100644 --- a/graph/context.js +++ b/graph/context.js @@ -71,6 +71,18 @@ class Context { // Bind the parent context. this.parent = parent; } + + /** + * + */ + static forSystem() { + const {models: {User}} = connectors; + + // Create the system user. + const user = new User({system: true}); + + return new Context({user}); + } } module.exports = Context; diff --git a/models/user.js b/models/user.js index f6797f94a..17e3a173d 100644 --- a/models/user.js +++ b/models/user.js @@ -5,6 +5,7 @@ const uuid = require('uuid'); const TagLinkSchema = require('./schema/tag_link'); const TokenSchema = require('./schema/token'); const can = require('../perms'); +const {get} = require('lodash'); // USER_ROLES is the array of roles that is permissible as a user role. const USER_ROLES = require('./enum/user_roles'); @@ -244,8 +245,6 @@ UserSchema.index({ background: true, }); -// TODO: Add indexes for searching the user collection. Needs product decision. - /** * returns true if a commenter is staff */ @@ -280,6 +279,22 @@ UserSchema.method('can', function(...actions) { return can(this, ...actions); }); +/** + * hasVerifiedEmail will return true if at least one of the local email accounts + * have their email verified. + */ +UserSchema.virtual('hasVerifiedEmail').get(function() { + return this.profiles + .filter(({provider}) => provider === 'local') + .some((profile) => { + const confirmedAt = get(profile, 'metadata.confirmed_at') || null; + + // 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. + return confirmedAt !== null; + }); +}); + /** * banned returns true when the user is currently banned, and sets the banned * status locally. diff --git a/package.json b/package.json index b1889158b..5675ca588 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "immutability-helper": "^2.2.0", "imports-loader": "^0.7.1", "inquirer": "^3.2.2", + "inquirer-autocomplete-prompt": "^0.12.1", "ioredis": "3.1.4", "joi": "^10.6.0", "json-loader": "^0.5.7", diff --git a/perms/reducers/index.js b/perms/reducers/index.js index 70761990d..933be8329 100644 --- a/perms/reducers/index.js +++ b/perms/reducers/index.js @@ -11,6 +11,13 @@ module.exports = [ return false; } }, + (user) => { + + // System users can do everything! + if (user.system === true) { + return true; + } + }, query, mutation, subscription, diff --git a/services/users.js b/services/users.js index f2dfee166..c9b0a73fd 100644 --- a/services/users.js +++ b/services/users.js @@ -359,35 +359,6 @@ class UsersService { ); } - /** - * Merges two users together by taking all the profiles on a given user and - * pushing them into the source user followed by deleting the destination user's - * user account. This will - * not merge the roles associated with the source user. - * @param {String} dstUserID id of the user to which is the target of the merge - * @param {String} srcUserID id of the user to which is the source of the merge - * @return {Promise} resolves when the users are merged - */ - static mergeUsers(dstUserID, srcUserID) { - let srcUser, dstUser; - - return Promise.all([ - UserModel.findOne({id: dstUserID}).exec(), - UserModel.findOne({id: srcUserID}).exec(), - ]) - .then((users) => { - dstUser = users[0]; - srcUser = users[1]; - - srcUser.profiles.forEach((profile) => { - dstUser.profiles.push(profile); - }); - - return srcUser.remove(); - }) - .then(() => dstUser.save()); - } - static castUsername(username) { return username.replace(/ /g, '_').replace(/[^a-zA-Z_]/g, ''); } diff --git a/yarn.lock b/yarn.lock index 03e59c6a1..2448dd1d6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -183,6 +183,10 @@ ansi-escapes@^1.1.0: version "1.4.0" resolved "https://registry.yarnpkg.com/ansi-escapes/-/ansi-escapes-1.4.0.tgz#d3a8a83b319aa67793662b13e761c7911422306e" +ansi-escapes@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/ansi-escapes/-/ansi-escapes-2.0.0.tgz#5bae52be424878dd9783e8910e3fc2922e83c81b" + ansi-escapes@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/ansi-escapes/-/ansi-escapes-3.0.0.tgz#ec3e8b4e9f8064fc02c3ac9b65f1c275bda8ef92" @@ -4237,6 +4241,16 @@ ini@^1.3.4, ini@~1.3.0: version "1.3.5" resolved "https://registry.yarnpkg.com/ini/-/ini-1.3.5.tgz#eee25f56db1c9ec6085e0c22778083f596abf927" +inquirer-autocomplete-prompt@^0.12.1: + version "0.12.1" + resolved "https://registry.yarnpkg.com/inquirer-autocomplete-prompt/-/inquirer-autocomplete-prompt-0.12.1.tgz#17b20145fcd656634555ad5645727bd0fe816c57" + dependencies: + ansi-escapes "^3.0.0" + chalk "^2.0.0" + figures "^2.0.0" + inquirer "3.2.0" + run-async "^2.3.0" + inquirer-confirm@0.2.2: version "0.2.2" resolved "https://registry.yarnpkg.com/inquirer-confirm/-/inquirer-confirm-0.2.2.tgz#6f406d037bf9d9e455ef0f953929f357fe9a8848" @@ -4275,6 +4289,25 @@ inquirer@0.8.2: rx "^2.4.3" through "^2.3.6" +inquirer@3.2.0: + version "3.2.0" + resolved "https://registry.yarnpkg.com/inquirer/-/inquirer-3.2.0.tgz#45b44c2160c729d7578c54060b3eed94487bb42b" + dependencies: + ansi-escapes "^2.0.0" + chalk "^2.0.0" + cli-cursor "^2.1.0" + cli-width "^2.0.0" + external-editor "^2.0.4" + figures "^2.0.0" + lodash "^4.3.0" + mute-stream "0.0.7" + run-async "^2.2.0" + rx-lite "^4.0.8" + rx-lite-aggregates "^4.0.8" + string-width "^2.1.0" + strip-ansi "^4.0.0" + through "^2.3.6" + inquirer@3.3.0, inquirer@^3.0.6, inquirer@^3.2.2: version "3.3.0" resolved "https://registry.yarnpkg.com/inquirer/-/inquirer-3.3.0.tgz#9dd2f2ad765dcab1ff0443b491442a20ba227dc9" @@ -8281,7 +8314,7 @@ run-async@^0.1.0: dependencies: once "^1.3.0" -run-async@^2.2.0: +run-async@^2.2.0, run-async@^2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/run-async/-/run-async-2.3.0.tgz#0371ab4ae0bdd720d4166d7dfda64ff7a445a6c0" dependencies: From c07ec23a2020fdf1d9b9486cfb7243aeca010aeb Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 5 Jan 2018 16:06:36 -0700 Subject: [PATCH 17/26] hardened configuration - moved env rewrite + dotenv code to single location - fixed bug with system flag on user --- bin/cli | 52 +++++++++++++++++++++----------------------- bin/cli-assets | 2 +- bin/cli-jobs | 2 +- bin/cli-migration | 2 +- bin/cli-plugins | 1 + bin/cli-serve | 2 +- bin/cli-settings | 2 +- bin/cli-setup | 2 +- bin/cli-token | 2 +- bin/cli-users | 4 ++++ bin/cli-verify | 2 +- bin/util.js | 4 ++++ config.js | 13 ++--------- models/user.js | 8 +++++++ package.json | 1 + services/env.js | 11 ++++++++++ services/mongoose.js | 10 ++++----- yarn.lock | 2 +- 18 files changed, 70 insertions(+), 52 deletions(-) create mode 100644 services/env.js diff --git a/bin/cli b/bin/cli index e01c9cd64..707f41032 100755 --- a/bin/cli +++ b/bin/cli @@ -4,6 +4,7 @@ * Module dependencies. */ +require('./util'); const program = require('commander'); const {head, map} = require('lodash'); const Matcher = require('did-you-mean'); @@ -18,40 +19,37 @@ program .command('users', 'work with the application auth') .command('migration', 'provides utilities for migrating the database') .command('verify', 'provides utilities for performing data verification') - .command( - 'plugins', - 'provides utilities for interacting with the plugin system' - ) + .command('plugins', 'provides utilities for interacting with the plugin system') .parse(process.argv); // If the command wasn't found, output help. -const cmds = map(program.commands, '_name'); -const cmd = head(program.args); -if (!cmds.includes(cmd)) { - const m = new Matcher(cmds); - const similarCMDs = m.list(cmd); +const commands = map(program.commands, '_name'); +const command = head(program.args); +if (!commands.includes(command)) { + const m = new Matcher(commands); + const similarCommands = m.list(command); - console.error(`cli '${cmd}' is not a talk cli command. See 'cli --help'.`); - if (similarCMDs.length > 0) { - const sc = similarCMDs.map(({value}) => `\t${value}\n`).join(''); + console.error(`cli '${command}' is not a talk cli command. See 'cli --help'.`); + if (similarCommands.length > 0) { + const sc = similarCommands.map(({value}) => `\t${value}\n`).join(''); console.error(`\nThe most similar commands are\n${sc}`); } process.exit(1); } -/** - * When this provess exists, check to see if we have a running command, if we do - * check to see if it is still running. If it is, then kill it with a SIGINT - * signal. This is for the use case where we want to kill the process that is - * labled with the PID written out by the parent process. - */ -process.once('exit', () => { - if ( +// /** +// * When this process exists, check to see if we have a running command, if we do +// * check to see if it is still running. If it is, then kill it with a SIGINT +// * signal. This is for the use case where we want to kill the process that is +// * labeled with the PID written out by the parent process. +// */ +// process.once('exit', () => { +// if ( - // program.runningCommand && - program.runningCommand.killed === false && - program.runningCommand.exitCode === null - ) { - program.runningCommand.kill('SIGINT'); - } -}); +// // program.runningCommand && +// program.runningCommand.killed === false && +// program.runningCommand.exitCode === null +// ) { +// program.runningCommand.kill('SIGINT'); +// } +// }); diff --git a/bin/cli-assets b/bin/cli-assets index 456f16877..5c1780c5d 100755 --- a/bin/cli-assets +++ b/bin/cli-assets @@ -4,6 +4,7 @@ * Module dependencies. */ +const util = require('./util'); const program = require('commander'); const parseDuration = require('ms'); const Table = require('cli-table'); @@ -12,7 +13,6 @@ const CommentModel = require('../models/comment'); const AssetsService = require('../services/assets'); const mongoose = require('../services/mongoose'); const scraper = require('../services/scraper'); -const util = require('./util'); const inquirer = require('inquirer'); // Register the shutdown criteria. diff --git a/bin/cli-jobs b/bin/cli-jobs index 345b22b56..26eeab147 100755 --- a/bin/cli-jobs +++ b/bin/cli-jobs @@ -4,10 +4,10 @@ * Module dependencies. */ +const util = require('./util'); 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'); diff --git a/bin/cli-migration b/bin/cli-migration index f730b8fbb..18a8c012a 100755 --- a/bin/cli-migration +++ b/bin/cli-migration @@ -4,8 +4,8 @@ * Module dependencies. */ -const program = require('commander'); const util = require('./util'); +const program = require('commander'); const inquirer = require('inquirer'); const mongoose = require('../services/mongoose'); const MigrationService = require('../services/migration'); diff --git a/bin/cli-plugins b/bin/cli-plugins index 8217b023f..4f293b0c3 100755 --- a/bin/cli-plugins +++ b/bin/cli-plugins @@ -7,6 +7,7 @@ // Interface heavily inspired by the yarn package manager: // https://yarnpkg.com/ +require('./util'); const program = require('commander'); const inquirer = require('inquirer'); diff --git a/bin/cli-serve b/bin/cli-serve index cc00f84ed..3e356d829 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -1,7 +1,7 @@ #!/usr/bin/env node -const program = require('commander'); const util = require('./util'); +const program = require('commander'); const serve = require('../serve'); //============================================================================== diff --git a/bin/cli-settings b/bin/cli-settings index 481e5b2bf..6ffacbeaf 100755 --- a/bin/cli-settings +++ b/bin/cli-settings @@ -1,10 +1,10 @@ #!/usr/bin/env node +const util = require('./util'); const program = require('commander'); const inquirer = require('inquirer'); const mongoose = require('../services/mongoose'); const SettingsService = require('../services/settings'); -const util = require('./util'); // Register the shutdown criteria. util.onshutdown([() => mongoose.disconnect()]); diff --git a/bin/cli-setup b/bin/cli-setup index 35521e05a..0f14add55 100755 --- a/bin/cli-setup +++ b/bin/cli-setup @@ -4,6 +4,7 @@ * Module dependencies. */ +const util = require('./util'); const program = require('commander'); const inquirer = require('inquirer'); const mongoose = require('../services/mongoose'); @@ -12,7 +13,6 @@ const MODERATION_OPTIONS = require('../models/enum/moderation_options'); const SettingsService = require('../services/settings'); const SetupService = require('../services/setup'); const UsersService = require('../services/users'); -const util = require('./util'); const errors = require('../errors'); // Register the shutdown criteria. diff --git a/bin/cli-token b/bin/cli-token index 87d955df9..dc1b09e6f 100755 --- a/bin/cli-token +++ b/bin/cli-token @@ -4,10 +4,10 @@ * Module dependencies. */ +const util = require('./util'); const program = require('commander'); const mongoose = require('../services/mongoose'); const TokensService = require('../services/tokens'); -const util = require('./util'); const Table = require('cli-table'); // Register the shutdown criteria. diff --git a/bin/cli-users b/bin/cli-users index b80af1fab..71c51e44d 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -156,6 +156,10 @@ async function searchUsers() { throw errors[0]; } + if (data.users === null) { + return []; + } + return data.users.nodes.map((user) => { const emails = user.profiles .filter(({provider}) => provider === 'local') diff --git a/bin/cli-verify b/bin/cli-verify index 9a4c30ab1..5a55f2bd2 100755 --- a/bin/cli-verify +++ b/bin/cli-verify @@ -4,9 +4,9 @@ * Module dependencies. */ +const util = require('./util'); const program = require('commander'); const mongoose = require('../services/mongoose'); -const util = require('./util'); const databaseVerifications = require('./verifications/database'); // Register the shutdown criteria. diff --git a/bin/util.js b/bin/util.js index abcbeaade..546607c20 100644 --- a/bin/util.js +++ b/bin/util.js @@ -1,3 +1,7 @@ + +// Setup the environment. +require('../services/env'); + const debug = require('debug')('talk:util'); const util = module.exports = {}; diff --git a/config.js b/config.js index e617b93e8..954d7a8df 100644 --- a/config.js +++ b/config.js @@ -2,17 +2,8 @@ // application. All defaults are assumed here, validation should also be // completed here. -// Perform rewrites to the runtime environment variables based on the contents -// of the process.env.REWRITE_ENV if it exists. This is done here as it is the -// entrypoint for the entire applications configuration. -require('env-rewrite').rewrite(); - -if (process.env.NODE_ENV !== 'test') { - - // Apply all the configuration provided in the .env file if it isn't already - // in the environment. - require('dotenv').config(); -} +// Setup the environment. +require('./services/env'); const uniq = require('lodash/uniq'); const ms = require('ms'); diff --git a/models/user.js b/models/user.js index 17e3a173d..37edf4dde 100644 --- a/models/user.js +++ b/models/user.js @@ -295,6 +295,14 @@ UserSchema.virtual('hasVerifiedEmail').get(function() { }); }); +UserSchema.virtual('system') + .get(function() { + return this._system; + }) + .set(function(system) { + this._system = system; + }); + /** * banned returns true when the user is currently banned, and sets the banned * status locally. diff --git a/package.json b/package.json index 5675ca588..7c8a0f9a0 100644 --- a/package.json +++ b/package.json @@ -173,6 +173,7 @@ "snake-case": "2.1.0", "style-loader": "^0.16.0", "subscriptions-transport-ws": "^0.7.2", + "supports-color": "^4", "timeago.js": "^2.0.3", "timekeeper": "^1.0.0", "tlds": "^1.196.0", diff --git a/services/env.js b/services/env.js new file mode 100644 index 000000000..a9feb67a8 --- /dev/null +++ b/services/env.js @@ -0,0 +1,11 @@ +// Perform rewrites to the runtime environment variables based on the contents +// of the process.env.REWRITE_ENV if it exists. This is done here as it is the +// entrypoint for the entire applications configuration. +require('env-rewrite').rewrite(); + +if (process.env.NODE_ENV !== 'test') { + + // Apply all the configuration provided in the .env file if it isn't already + // in the environment. + require('dotenv').config(); +} diff --git a/services/mongoose.js b/services/mongoose.js index 747df2d66..6f0759908 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -1,14 +1,14 @@ -const mongoose = require('mongoose'); -const debug = require('debug')('talk:db'); -const enabled = require('debug').enabled; -const queryDebugger = require('debug')('talk:db:query'); - const { MONGO_URL, WEBPACK, CREATE_MONGO_INDEXES, } = require('../config'); +const mongoose = require('mongoose'); +const debug = require('debug')('talk:db'); +const enabled = require('debug').enabled; +const queryDebugger = 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 diff --git a/yarn.lock b/yarn.lock index 2448dd1d6..cbdddb034 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8997,7 +8997,7 @@ supports-color@^3.1.0, supports-color@^3.1.2, supports-color@^3.2.3: dependencies: has-flag "^1.0.0" -supports-color@^4.0.0, supports-color@^4.4.0: +supports-color@^4, supports-color@^4.0.0, supports-color@^4.4.0: version "4.5.0" resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-4.5.0.tgz#be7a0de484dec5c5cddf8b3d59125044912f635b" dependencies: From 717d5e656677202059fadfeda7ce9c2b9b8c24b9 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 8 Jan 2018 14:43:14 +0100 Subject: [PATCH 18/26] Rename UnBan and UnSuspend --- client/coral-admin/src/graphql/index.js | 4 ++-- client/coral-framework/graphql/fragments.js | 4 ++-- client/coral-framework/graphql/mutations.js | 8 ++++---- graph/typeDefs.graphql | 14 +++++++------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/client/coral-admin/src/graphql/index.js b/client/coral-admin/src/graphql/index.js index ed4e772cf..80573a2ad 100644 --- a/client/coral-admin/src/graphql/index.js +++ b/client/coral-admin/src/graphql/index.js @@ -56,7 +56,7 @@ export default { proxy.writeFragment({fragment: userStatusFragment, id: fragmentId, data: updated}); } }), - UnSuspendUser: ({variables: {input: {id}}}) => ({ + UnsuspendUser: ({variables: {input: {id}}}) => ({ update: (proxy) => { const fragmentId = `User_${id}`; const data = proxy.readFragment({fragment: userStatusFragment, id: fragmentId}); @@ -92,7 +92,7 @@ export default { proxy.writeFragment({fragment: userStatusFragment, id: fragmentId, data: updated}); } }), - UnBanUser: ({variables: {input: {id}}}) => ({ + UnbanUser: ({variables: {input: {id}}}) => ({ update: (proxy) => { const fragmentId = `User_${id}`; const data = proxy.readFragment({fragment: userStatusFragment, id: fragmentId}); diff --git a/client/coral-framework/graphql/fragments.js b/client/coral-framework/graphql/fragments.js index 26073676b..9d6ff3f72 100644 --- a/client/coral-framework/graphql/fragments.js +++ b/client/coral-framework/graphql/fragments.js @@ -6,11 +6,11 @@ export default { 'SetUserRoleResponse', 'ChangeUsernameResponse', 'BanUsersResponse', - 'UnBanUserResponse', + 'UnbanUserResponse', 'SetUserSuspensionStatusResponse', 'SetCommentStatusResponse', 'SetUsernameStatusResponse', - 'UnSuspendUserResponse', + 'UnsuspendUserResponse', 'SuspendUserResponse', 'CreateCommentResponse', 'CreateFlagResponse', diff --git a/client/coral-framework/graphql/mutations.js b/client/coral-framework/graphql/mutations.js index 0086d9a00..27b10669b 100644 --- a/client/coral-framework/graphql/mutations.js +++ b/client/coral-framework/graphql/mutations.js @@ -179,9 +179,9 @@ export const withSuspendUser = withMutation( export const withUnsuspendUser = withMutation( gql` - mutation UnSuspendUser($input: UnSuspendUserInput!) { + mutation UnsuspendUser($input: UnsuspendUserInput!) { unsuspendUser(input: $input) { - ...UnSuspendUserResponse + ...UnsuspendUserResponse } } `, { @@ -275,9 +275,9 @@ export const withBanUser = withMutation( export const withUnbanUser = withMutation( gql` - mutation UnBanUser($input: UnBanUserInput!) { + mutation UnbanUser($input: UnbanUserInput!) { unbanUser(input: $input) { - ...UnBanUserResponse + ...UnbanUserResponse } } `, { diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index e39238d4e..002318bc3 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -1314,7 +1314,7 @@ input BanUserInput { message: String! } -input UnBanUserInput { +input UnbanUserInput { id: ID! } @@ -1322,7 +1322,7 @@ type BanUsersResponse implements Response { errors: [UserError!] } -type UnBanUserResponse implements Response { +type UnbanUserResponse implements Response { errors: [UserError!] } @@ -1336,13 +1336,13 @@ type SuspendUserResponse implements Response { errors: [UserError!] } -input UnSuspendUserInput { +input UnsuspendUserInput { id: ID! } -# UnSuspendUserResponse is the response returned with possibly some +# UnsuspendUserResponse is the response returned with possibly some # errors relating to the suspend action attempt. -type UnSuspendUserResponse implements Response { +type UnsuspendUserResponse implements Response { # An array of errors relating to the mutation that occurred. errors: [UserError!] @@ -1396,7 +1396,7 @@ type RootMutation { # Sets the suspension status on a given user. Requires the `MODERATOR` role. # Mutation is restricted. - unsuspendUser(input: UnSuspendUserInput!): UnSuspendUserResponse + unsuspendUser(input: UnsuspendUserInput!): UnsuspendUserResponse # Sets the ban status on a given user. Requires the `MODERATOR` role. # Mutation is restricted. @@ -1404,7 +1404,7 @@ type RootMutation { # Sets the ban status on a given user. Requires the `MODERATOR` role. # Mutation is restricted. - unbanUser(input: UnBanUserInput!): UnBanUserResponse + unbanUser(input: UnbanUserInput!): UnbanUserResponse # Sets the username status on a given user to `APPROVED`. Requires the # `MODERATOR` role. Mutation is restricted. From be72e2effd0c51dd51fe5c6b74c93d2d25bdfd39 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 8 Jan 2018 15:01:09 +0100 Subject: [PATCH 19/26] Remove defunct refetch detection workaround --- client/coral-embed-stream/src/reducers/embed.js | 17 ----------------- .../src/tabs/stream/containers/Stream.js | 4 ++-- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/client/coral-embed-stream/src/reducers/embed.js b/client/coral-embed-stream/src/reducers/embed.js index 06073c54e..8e061827d 100644 --- a/client/coral-embed-stream/src/reducers/embed.js +++ b/client/coral-embed-stream/src/reducers/embed.js @@ -15,23 +15,6 @@ export default function stream(state = initialState, action) { activeTab: action.tab, previousTab: state.activeTab, }; - case 'APOLLO_QUERY_INIT': - if (action.queryString.indexOf('query CoralEmbedStream_Embed(') >= 0) { - return { - ...state, - refetching: action.isRefetch ? true : state.refetching, - refetchRequestId: action.isRefetch ? action.requestId : state.refetchRequestId, - }; - } - return state; - case 'APOLLO_QUERY_RESULT': - if (action.operationName === 'CoralEmbedStream_Embed') { - return { - ...state, - refetching: action.requestId === state.refetchRequestId ? false : state.refetching, - }; - } - return state; default: return state; } diff --git a/client/coral-embed-stream/src/tabs/stream/containers/Stream.js b/client/coral-embed-stream/src/tabs/stream/containers/Stream.js index 781fdcb99..62a288a15 100644 --- a/client/coral-embed-stream/src/tabs/stream/containers/Stream.js +++ b/client/coral-embed-stream/src/tabs/stream/containers/Stream.js @@ -211,7 +211,8 @@ class StreamContainer extends React.Component { return ; } - const streamLoading = this.props.refetching || this.props.data.loading; + // @TODO: Detect refetch when we have apollo 2.0. + const streamLoading = this.props.data.loading; return ( ({ auth: state.auth, - refetching: state.embed.refetching, activeReplyBox: state.stream.activeReplyBox, commentId: state.stream.commentId, assetId: state.stream.assetId, From 791025b153edf3a2bbbe5b5c7124fd7ac20699de Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 8 Jan 2018 15:48:49 +0100 Subject: [PATCH 20/26] Fix change username not displaying errors --- client/coral-embed-stream/src/components/ChangeUsername.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/components/ChangeUsername.js b/client/coral-embed-stream/src/components/ChangeUsername.js index 6fa7d3c0c..1c21eeb94 100644 --- a/client/coral-embed-stream/src/components/ChangeUsername.js +++ b/client/coral-embed-stream/src/components/ChangeUsername.js @@ -6,6 +6,7 @@ import styles from './ChangeUsername.css'; import {Button} from 'coral-ui'; import validate from 'coral-framework/helpers/validate'; import RestrictedMessageBox from 'coral-framework/components/RestrictedMessageBox'; +import {forEachError} from 'plugin-api/beta/client/utils'; class ChangeUsername extends Component { @@ -31,7 +32,9 @@ class ChangeUsername extends Component { changeUsername(user.id, username) .then(() => location.reload()) .catch((error) => { - this.setState({alert: t(`error.${error.translation_key}`)}); + let errorMsg = ''; + forEachError(error, ({msg}) => errorMsg = errorMsg ? `${errorMsg}, ${msg}` : msg); + this.setState({alert: errorMsg}); }); } else { this.setState({alert: t('framework.edit_name.error')}); From ec852601a43b560a62d8f15df0c32f6c65cb568f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 8 Jan 2018 15:50:41 +0100 Subject: [PATCH 21/26] Move ChangeUsername to the appropiate tab --- .../src/{ => tabs/stream}/components/ChangeUsername.css | 0 .../src/{ => tabs/stream}/components/ChangeUsername.js | 0 client/coral-embed-stream/src/tabs/stream/components/Stream.js | 2 +- .../src/{ => tabs/stream}/containers/ChangeUsername.js | 0 4 files changed, 1 insertion(+), 1 deletion(-) rename client/coral-embed-stream/src/{ => tabs/stream}/components/ChangeUsername.css (100%) rename client/coral-embed-stream/src/{ => tabs/stream}/components/ChangeUsername.js (100%) rename client/coral-embed-stream/src/{ => tabs/stream}/containers/ChangeUsername.js (100%) diff --git a/client/coral-embed-stream/src/components/ChangeUsername.css b/client/coral-embed-stream/src/tabs/stream/components/ChangeUsername.css similarity index 100% rename from client/coral-embed-stream/src/components/ChangeUsername.css rename to client/coral-embed-stream/src/tabs/stream/components/ChangeUsername.css diff --git a/client/coral-embed-stream/src/components/ChangeUsername.js b/client/coral-embed-stream/src/tabs/stream/components/ChangeUsername.js similarity index 100% rename from client/coral-embed-stream/src/components/ChangeUsername.js rename to client/coral-embed-stream/src/tabs/stream/components/ChangeUsername.js diff --git a/client/coral-embed-stream/src/tabs/stream/components/Stream.js b/client/coral-embed-stream/src/tabs/stream/components/Stream.js index 16eb7433c..36a4666aa 100644 --- a/client/coral-embed-stream/src/tabs/stream/components/Stream.js +++ b/client/coral-embed-stream/src/tabs/stream/components/Stream.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import {StreamError} from './StreamError'; import Comment from '../containers/Comment'; import BannedAccount from '../../../components/BannedAccount'; -import ChangeUsername from '../../../containers/ChangeUsername'; +import ChangeUsername from '../containers/ChangeUsername'; import Slot from 'coral-framework/components/Slot'; import InfoBox from 'talk-plugin-infobox/InfoBox'; import {can} from 'coral-framework/services/perms'; diff --git a/client/coral-embed-stream/src/containers/ChangeUsername.js b/client/coral-embed-stream/src/tabs/stream/containers/ChangeUsername.js similarity index 100% rename from client/coral-embed-stream/src/containers/ChangeUsername.js rename to client/coral-embed-stream/src/tabs/stream/containers/ChangeUsername.js From c3a5da3ae637b33aa85e5c38573f4fc0bd72f27b Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 8 Jan 2018 16:00:20 +0100 Subject: [PATCH 22/26] Fix live status updates --- client/coral-embed-stream/src/actions/auth.js | 5 +++ .../coral-embed-stream/src/constants/auth.js | 1 + .../src/containers/Embed.js | 19 +++++++-- .../coral-embed-stream/src/reducers/auth.js | 42 +++++-------------- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/client/coral-embed-stream/src/actions/auth.js b/client/coral-embed-stream/src/actions/auth.js index 64e29a31a..578ca9812 100644 --- a/client/coral-embed-stream/src/actions/auth.js +++ b/client/coral-embed-stream/src/actions/auth.js @@ -5,6 +5,11 @@ import {notify} from 'coral-framework/actions/notification'; import t from 'coral-framework/services/i18n'; import get from 'lodash/get'; +export const updateStatus = (status) => ({ + type: actions.UPDATE_STATUS, + status, +}); + export const showSignInDialog = () => ({ type: actions.SHOW_SIGNIN_DIALOG, }); diff --git a/client/coral-embed-stream/src/constants/auth.js b/client/coral-embed-stream/src/constants/auth.js index bb2ea6c15..3187873fe 100644 --- a/client/coral-embed-stream/src/constants/auth.js +++ b/client/coral-embed-stream/src/constants/auth.js @@ -54,3 +54,4 @@ export const SET_REQUIRE_EMAIL_VERIFICATION = 'SET_REQUIRE_EMAIL_VERIFICATION'; export const SET_REDIRECT_URI = 'SET_REDIRECT_URI'; export const RESET_SIGNIN_DIALOG = 'RESET_SIGNIN_DIALOG'; +export const UPDATE_STATUS = 'UPDATE_STATUS'; diff --git a/client/coral-embed-stream/src/containers/Embed.js b/client/coral-embed-stream/src/containers/Embed.js index e0b144d62..6b51bb99a 100644 --- a/client/coral-embed-stream/src/containers/Embed.js +++ b/client/coral-embed-stream/src/containers/Embed.js @@ -21,7 +21,14 @@ import t from 'coral-framework/services/i18n'; import PropTypes from 'prop-types'; import {setActiveTab} from '../actions/embed'; -const {logout, checkLogin, focusSignInDialog, blurSignInDialog, hideSignInDialog} = authActions; +const { + logout, + checkLogin, + focusSignInDialog, + blurSignInDialog, + hideSignInDialog, + updateStatus, +} = authActions; const {fetchAssetSuccess} = assetActions; class EmbedContainer extends React.Component { @@ -35,20 +42,23 @@ class EmbedContainer extends React.Component { if (props.auth.loggedIn) { const newSubscriptions = [{ document: USER_BANNED_SUBSCRIPTION, - updateQuery: () => { + updateQuery: (_, {subscriptionData: {data: {userBanned: {state}}}}) => { notify('info', t('your_account_has_been_banned')); + props.updateStatus(state.status); }, }, { document: USER_SUSPENDED_SUBSCRIPTION, - updateQuery: () => { + updateQuery: (_, {subscriptionData: {data: {userSuspended: {state}}}}) => { notify('info', t('your_account_has_been_suspended')); + props.updateStatus(state.status); }, }, { document: USERNAME_REJECTED_SUBSCRIPTION, - updateQuery: () => { + updateQuery: (_, {subscriptionData: {data: {usernameRejected: {state}}}}) => { notify('info', t('your_username_has_been_rejected')); + props.updateStatus(state.status); }, }]; @@ -260,6 +270,7 @@ const mapDispatchToProps = (dispatch) => focusSignInDialog, blurSignInDialog, hideSignInDialog, + updateStatus, }, dispatch ); diff --git a/client/coral-embed-stream/src/reducers/auth.js b/client/coral-embed-stream/src/reducers/auth.js index 156c53515..1f044a8ef 100644 --- a/client/coral-embed-stream/src/reducers/auth.js +++ b/client/coral-embed-stream/src/reducers/auth.js @@ -1,5 +1,6 @@ import * as actions from '../constants/auth'; import pym from 'coral-framework/services/pym'; +import merge from 'lodash/merge'; const initialState = { isLoading: false, @@ -227,38 +228,15 @@ export default function auth (state = initialState, action) { ...state, redirectUri: action.uri, }; - case 'APOLLO_SUBSCRIPTION_RESULT': - - // @TODO: These don't work anymore because apollo store has been decoupled - - if (action.operationName === 'UserBanned' && state.user.id === action.variables.user_id) { - return { - ...state, - user: { - ...state.user, - ...action.result.data.userBanned, - }, - }; - } - if (action.operationName === 'UserSuspended' && state.user.id === action.variables.user_id) { - return { - ...state, - user: { - ...state.user, - ...action.result.data.userSuspended, - }, - }; - } - if (action.operationName === 'UsernameRejected' && state.user.id === action.variables.user_id) { - return { - ...state, - user: { - ...state.user, - ...action.result.data.usernameRejected, - }, - }; - } - return state; + case actions.UPDATE_STATUS: { + return { + ...state, + user: { + ...state.user, + status: merge({}, state.user.status, action.status), + }, + }; + } default : return state; } From 99d66fef522e2d86a07b0eb329e76d241ac7540b Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Mon, 8 Jan 2018 11:27:25 -0500 Subject: [PATCH 23/26] Docs typo --- docs/_docs/03-02-product-guide-commenter-features.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_docs/03-02-product-guide-commenter-features.md b/docs/_docs/03-02-product-guide-commenter-features.md index 7f56880db..144bdb634 100644 --- a/docs/_docs/03-02-product-guide-commenter-features.md +++ b/docs/_docs/03-02-product-guide-commenter-features.md @@ -197,4 +197,4 @@ will see a message at the top of their streams stating this. ### Ban When a commenter has been banned, they will see a message at the top of their -streams staging this. +streams stating this. From 615be40b454a78c07985ad6583ebfe7eef25a1a7 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 8 Jan 2018 10:43:10 -0700 Subject: [PATCH 24/26] refactored mailer service with lodash! --- services/mailer.js | 44 ++++++++----------- services/users.js | 6 +-- .../graph/mutations/setUserBanStatus.js | 4 +- .../mutations/setUserSuspensionStatus.js | 4 +- test/server/services/users.js | 6 +-- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/services/mailer.js b/services/mailer.js index ecb65de8c..08f1d6c29 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,13 +1,12 @@ const debug = require('debug')('talk:services:mailer'); const nodemailer = require('nodemailer'); const kue = require('./kue'); +const i18n = require('./i18n'); const path = require('path'); -const fs = require('fs'); +const fs = require('fs-extra'); const _ = require('lodash'); const {TEMPLATE_LOCALS} = require('../middleware/staticTemplate'); -const i18n = require('./i18n'); - const { SMTP_HOST, SMTP_USERNAME, @@ -23,38 +22,29 @@ const templates = { }; // load the templates per request during development -templates.render = (name, format = 'txt', context) => new Promise((resolve, reject) => { +templates.render = async (name, format = 'txt', context) => { - // If we are in production mode, check the view cache. if (process.env.NODE_ENV === 'production') { - if (name in templates.data && format in templates.data[name]) { - let view = templates.data[name][format]; - return resolve(view(context)); + // If we are in production mode, check the view cache. + const view = _.get(templates.data, [name, format], null); + if (view !== null) { + return view(context); } } const filename = path.join(__dirname, 'email', [name, format, 'ejs'].join('.')); + const file = await fs.readFile(filename, 'utf8'); + const view = _.template(file); - fs.readFile(filename, (err, file) => { - if (err) { - return reject(err); - } - - let view = _.template(file); + if (process.env.NODE_ENV === 'production') { // If we are in production mode, fill the view cache. - if (process.env.NODE_ENV === 'production') { - if (!(name in templates.data)) { - templates.data[name] = {}; - } + _.set(templates.data, [name, format], view); + } - templates.data[name][format] = view; - } - - return resolve(view(context)); - }); -}); + return view(context); +}; const mailer = {}; @@ -103,7 +93,9 @@ mailer.task = new kue.Task({ */ mailer.send = async (options) => { if (!mailer.enabled) { - throw new Error('email is not enabled because required configuration is not available'); + const err = new Error('sending email is not enabled because required configuration is not available'); + console.warn(err); + return; } // Create the new locals object and attach the static locals and the i18n @@ -118,7 +110,7 @@ mailer.send = async (options) => { return templates.render(options.template, fmt, locals); })); - // Create the job. + // Create the job to send the email later. return mailer.task.create({ title: 'Mail', message: { diff --git a/services/users.js b/services/users.js index c9b0a73fd..33c6047fa 100644 --- a/services/users.js +++ b/services/users.js @@ -24,7 +24,7 @@ const RECAPTCHA_WINDOW = '10m'; // 10 minutes. const RECAPTCHA_INCORRECT_TRIGGER = 5; // after 3 incorrect attempts, recaptcha will be required. const ActionsService = require('./actions'); -const MailerService = require('./mailer'); +const mailer = require('./mailer'); const i18n = require('./i18n'); const Wordlist = require('./wordlist'); const DomainList = require('./domain_list'); @@ -423,7 +423,7 @@ class UsersService { redirectURI ); - return MailerService.send({ + return mailer.send({ template: 'email-confirm', locals: { token, @@ -449,7 +449,7 @@ class UsersService { to, }); - return MailerService.send(options); + return mailer.send(options); } static async changePassword(id, password) { diff --git a/test/server/graph/mutations/setUserBanStatus.js b/test/server/graph/mutations/setUserBanStatus.js index 37a6bac62..3b62ba327 100644 --- a/test/server/graph/mutations/setUserBanStatus.js +++ b/test/server/graph/mutations/setUserBanStatus.js @@ -5,7 +5,7 @@ const Context = require('../../../../graph/context'); const SettingsService = require('../../../../services/settings'); const UserModel = require('../../../../models/user'); const UsersService = require('../../../../services/users'); -const MailerService = require('../../../../services/mailer'); +const mailer = require('../../../../services/mailer'); const sinon = require('sinon'); const chai = require('chai'); @@ -22,7 +22,7 @@ describe('graph.mutations.banUser', () => { let spy; before(() => { - spy = sinon.spy(MailerService, 'send'); + spy = sinon.spy(mailer, 'send'); }); afterEach(() => { diff --git a/test/server/graph/mutations/setUserSuspensionStatus.js b/test/server/graph/mutations/setUserSuspensionStatus.js index 5ac46fe84..3a18ad00a 100644 --- a/test/server/graph/mutations/setUserSuspensionStatus.js +++ b/test/server/graph/mutations/setUserSuspensionStatus.js @@ -6,7 +6,7 @@ const Context = require('../../../../graph/context'); const SettingsService = require('../../../../services/settings'); const UserModel = require('../../../../models/user'); const UsersService = require('../../../../services/users'); -const MailerService = require('../../../../services/mailer'); +const mailer = require('../../../../services/mailer'); const sinon = require('sinon'); const chai = require('chai'); @@ -24,7 +24,7 @@ describe('graph.mutations.suspendUser', () => { let spy; before(() => { - spy = sinon.spy(MailerService, 'send'); + spy = sinon.spy(mailer, 'send'); }); afterEach(() => { diff --git a/test/server/services/users.js b/test/server/services/users.js index 343b73bdf..837ce9fa2 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -1,6 +1,6 @@ const UsersService = require('../../../services/users'); const SettingsService = require('../../../services/settings'); -const MailerService = require('../../../services/mailer'); +const mailer = require('../../../services/mailer'); const chai = require('chai'); chai.use(require('chai-as-promised')); @@ -29,11 +29,11 @@ describe('services.UsersService', () => { password: '3Coral!3' }]); - sinon.spy(MailerService, 'send'); + sinon.spy(mailer, 'send'); }); afterEach(() => { - MailerService.send.restore(); + mailer.send.restore(); }); describe('#findById()', () => { From 978e1c8d4ceae38e8b3f74622661dee0cf3ab9d2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 8 Jan 2018 11:04:29 -0700 Subject: [PATCH 25/26] fix to optional required param --- config.js | 26 +------------------------- services/mailer.js | 1 - 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/config.js b/config.js index 954d7a8df..6379b08f0 100644 --- a/config.js +++ b/config.js @@ -343,9 +343,7 @@ CONFIG.REDIS_CLIENT_CONFIG = JSON.parse(CONFIG.REDIS_CLIENT_CONFIG); */ CONFIG.RECAPTCHA_ENABLED = CONFIG.RECAPTCHA_SECRET && - CONFIG.RECAPTCHA_SECRET.length > 0 && - CONFIG.RECAPTCHA_PUBLIC && - CONFIG.RECAPTCHA_PUBLIC.length > 0; + CONFIG.RECAPTCHA_PUBLIC; debug( `reCAPTCHA is ${ @@ -355,26 +353,4 @@ debug( }` ); -//------------------------------------------------------------------------------ -// SMTP Server configuration -//------------------------------------------------------------------------------ - -CONFIG.EMAIL_ENABLED = - CONFIG.SMTP_FROM_ADDRESS && - CONFIG.SMTP_FROM_ADDRESS.length > 0 && - CONFIG.SMTP_USERNAME && - CONFIG.SMTP_USERNAME.length > 0 && - CONFIG.SMTP_PASSWORD && - CONFIG.SMTP_PASSWORD.length > 0 && - CONFIG.SMTP_HOST && - CONFIG.SMTP_HOST.length > 0; - -debug( - `Email is ${ - CONFIG.EMAIL_ENABLED - ? 'enabled' - : 'disabled, required config is not present' - }` -); - module.exports = CONFIG; diff --git a/services/mailer.js b/services/mailer.js index 08f1d6c29..f66fd8240 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -54,7 +54,6 @@ const mailer = {}; mailer.enabled = Boolean( SMTP_HOST && SMTP_USERNAME && - SMTP_PORT && SMTP_PASSWORD && SMTP_FROM_ADDRESS ) || process.env.NODE_ENV === 'test'; From 8545be307d01ac97fe3dab4406cf79217bb04aea Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 8 Jan 2018 11:17:11 -0700 Subject: [PATCH 26/26] removed snakecase --- bin/verifications/database/action_counts.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/verifications/database/action_counts.js b/bin/verifications/database/action_counts.js index 133c1a2e3..2aa47a9a7 100644 --- a/bin/verifications/database/action_counts.js +++ b/bin/verifications/database/action_counts.js @@ -3,7 +3,6 @@ const CommentModel = require('../../../models/comment'); const ActionsService = require('../../../services/actions'); const {arrayJoinBy} = require('../../../graph/loaders/util'); const {get} = require('lodash'); -const sc = require('snake-case'); const debug = require('debug')('talk:cli:verify'); const MODELS = [ @@ -38,8 +37,8 @@ async function processBatch(Model, documents) { } // And we generate the group id. - const ACTION_TYPE = sc(actionSummary.action_type.toLowerCase()); - const GROUP_ID = sc(actionSummary.group_id.toLowerCase()); + const ACTION_TYPE = actionSummary.action_type.toLowerCase(); + const GROUP_ID = actionSummary.group_id.toLowerCase(); if (GROUP_ID.length <= 0) { continue;