From 60d47931112cb98e3efb644bdf57dbb1bee0659e Mon Sep 17 00:00:00 2001 From: gaba Date: Fri, 2 Dec 2016 16:12:24 -0800 Subject: [PATCH 01/14] Adds a variable to the possible translations. --- client/coral-framework/modules/i18n/i18n.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/coral-framework/modules/i18n/i18n.js b/client/coral-framework/modules/i18n/i18n.js index afe8606d1..a4a22f5b8 100644 --- a/client/coral-framework/modules/i18n/i18n.js +++ b/client/coral-framework/modules/i18n/i18n.js @@ -46,9 +46,11 @@ class i18n { * it takes a string with the translation key and returns * the translation value or the key itself if not found * it works with nested translations (my.page.title) + * + * the second parameter is optional and replaces a variable marked by {0} in the translation. */ - this.t = (key) => { + this.t = (key, att) => { const arr = key.split('.'); let translation = this.translations; try { @@ -58,8 +60,9 @@ class i18n { return key; } - const val = String(translation); + let val = String(translation); if (val) { + val = val.replace('/\{0\/g', att); return val; } else { console.warn(`${key} language key not set`); From 31c87cb93f018357eb81da5789fee0b36cab16f9 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Tue, 6 Dec 2016 14:17:01 -1000 Subject: [PATCH 02/14] make the translation function accept n arguments. ninja bugfix to permalink --- .../coral-embed-stream/src/CommentStream.js | 2 +- client/coral-framework/modules/i18n/i18n.js | 26 ++++++++----------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/client/coral-embed-stream/src/CommentStream.js b/client/coral-embed-stream/src/CommentStream.js index 0fda9444d..bb1d4e6e0 100644 --- a/client/coral-embed-stream/src/CommentStream.js +++ b/client/coral-embed-stream/src/CommentStream.js @@ -70,7 +70,7 @@ class CommentStream extends Component { const path = /https?\:\/\/([^?#]+)/.exec(this.pym.parentUrl); this.props.getStream(path[1] || window.location); - this.path = path; + this.path = path[1] || window.location; this.pym.sendMessage('childReady'); } diff --git a/client/coral-framework/modules/i18n/i18n.js b/client/coral-framework/modules/i18n/i18n.js index a4a22f5b8..2bab752db 100644 --- a/client/coral-framework/modules/i18n/i18n.js +++ b/client/coral-framework/modules/i18n/i18n.js @@ -1,5 +1,7 @@ import timeago from 'timeago.js'; import esTA from '../../../../node_modules/timeago.js/locales/es'; +import has from 'lodash/has'; +import get from 'lodash/get'; /** * Default locales, this should be overriden by config file @@ -47,23 +49,17 @@ class i18n { * the translation value or the key itself if not found * it works with nested translations (my.page.title) * - * the second parameter is optional and replaces a variable marked by {0} in the translation. + * any extra parameters are optional and replace a variable marked by {0}, {1}, etc in the translation. */ - this.t = (key, att) => { - const arr = key.split('.'); - let translation = this.translations; - try { - for (let i = 0; i < arr.length; i++) {translation = translation[arr[i]];} - } catch (error) { - console.warn(`${key} language key not set`); - return key; - } - - let val = String(translation); - if (val) { - val = val.replace('/\{0\/g', att); - return val; + this.t = (key, ...replacements) => { + if (has(this.translations, key)) { + let translation = get(this.translations, key); + // replace any {n} with the arguments passed to this method + replacements.forEach((str, i) => { + translation = translation.replace(`{${i}}`, str); + }); + return translation; } else { console.warn(`${key} language key not set`); return key; From f2ded3f2a7daabbc6041671471cded0da57653cb Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Tue, 6 Dec 2016 14:34:27 -1000 Subject: [PATCH 03/14] replace things multiple times --- client/coral-framework/modules/i18n/i18n.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-framework/modules/i18n/i18n.js b/client/coral-framework/modules/i18n/i18n.js index 2bab752db..1d81e885c 100644 --- a/client/coral-framework/modules/i18n/i18n.js +++ b/client/coral-framework/modules/i18n/i18n.js @@ -57,7 +57,7 @@ class i18n { let translation = get(this.translations, key); // replace any {n} with the arguments passed to this method replacements.forEach((str, i) => { - translation = translation.replace(`{${i}}`, str); + translation = translation.replace(new RegExp(`\\{${i}\\}`, 'g'), str); }); return translation; } else { From 12011965667963c57bd7a4bd0f7983555aac2c65 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Tue, 6 Dec 2016 15:51:22 -1000 Subject: [PATCH 04/14] un-sneaking in some stuff --- client/coral-embed-stream/src/CommentStream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/CommentStream.js b/client/coral-embed-stream/src/CommentStream.js index bb1d4e6e0..0fda9444d 100644 --- a/client/coral-embed-stream/src/CommentStream.js +++ b/client/coral-embed-stream/src/CommentStream.js @@ -70,7 +70,7 @@ class CommentStream extends Component { const path = /https?\:\/\/([^?#]+)/.exec(this.pym.parentUrl); this.props.getStream(path[1] || window.location); - this.path = path[1] || window.location; + this.path = path; this.pym.sendMessage('childReady'); } From 260d1bd9f44f89a1985db2df7637db5d89336075 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Tue, 6 Dec 2016 16:29:40 -1000 Subject: [PATCH 05/14] restoring the scroll to comment functionality --- .../coral-embed-stream/src/CommentStream.js | 21 +++++++++++++++++-- client/coral-embed-stream/style/default.css | 1 - 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/client/coral-embed-stream/src/CommentStream.js b/client/coral-embed-stream/src/CommentStream.js index 0fda9444d..c7b7b2412 100644 --- a/client/coral-embed-stream/src/CommentStream.js +++ b/client/coral-embed-stream/src/CommentStream.js @@ -67,12 +67,29 @@ class CommentStream extends Component { // Set up messaging between embedded Iframe an parent component this.pym = new Pym.Child({polling: 100}); - const path = /https?\:\/\/([^?#]+)/.exec(this.pym.parentUrl); + const path = this.pym.parentUrl.split('#')[0]; - this.props.getStream(path[1] || window.location); + this.props.getStream(path || window.location); this.path = path; this.pym.sendMessage('childReady'); + + this.pym.onMessage('DOMContentLoaded', hash => { + const commentId = hash.replace('#', 'c_'); + let count = 0; + const interval = setInterval(() => { + if (document.getElementById(commentId)) { + window.clearInterval(interval); + this.pym.scrollParentToChildEl(commentId); + } + + if (++count > 100) { // ~10 seconds + // give up waiting for the comments to load. + // it would be weird for the page to jump after that long. + window.clearInterval(interval); + } + }, 100); + }); } render () { diff --git a/client/coral-embed-stream/style/default.css b/client/coral-embed-stream/style/default.css index 59efc3dc8..688fc78f6 100644 --- a/client/coral-embed-stream/style/default.css +++ b/client/coral-embed-stream/style/default.css @@ -113,7 +113,6 @@ hr { /* Comment styles */ .comment { - position: relative; margin-bottom: 10px; position: relative; } From 61c0af93e3684fe86255997ac648b1f10bf93dc4 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 10:28:01 -1000 Subject: [PATCH 06/14] add some debug statements so we can figure out why pw reset doesn't work in staging --- models/user.js | 13 +++++++++++++ routes/api/user/index.js | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/models/user.js b/models/user.js index 336486a54..8c478fbb4 100644 --- a/models/user.js +++ b/models/user.js @@ -2,6 +2,7 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); +const debug = require('debug')('talk:users'); // SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run // through during the salting process. @@ -435,23 +436,35 @@ UserService.findPublicByIdArray = (ids) => { */ UserService.createPasswordResetToken = function (email) { if (!email || typeof email !== 'string') { + debug('the email was missing to createPasswordResetToken. you sent %s', email); return Promise.reject('email is required when creating a JWT for resetting passord'); } + email = email.toLowerCase(); + debug('about to look up user by email %s', email); + return UserModel.findOne({profiles: {$elemMatch: {id: email}}}) .then(user => { if (user === null) { + debug('user was null on createPasswordResetToken lookup'); // since we don't want to reveal that the email does/doesn't exist // just go ahead and resolve the Promise with null and check in the endpoint return Promise.resolve(null); } + debug('found the user! %s going to create the token', JSON.stringify(user, null 2)); + const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; + + debug('created payload %s', JSON.stringify(payload)); + const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); + debug('successfully created the token %s', token); + return token; }); }; diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 1765809ad..05ebf7b61 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -8,6 +8,7 @@ const path = require('path'); const resetEmailFile = fs.readFileSync(path.resolve(__dirname, '../../../views/password-reset-email.ejs')); const resetEmailTemplate = ejs.compile(resetEmailFile.toString()); const authorization = require('../../../middleware/authorization'); +const debug = require('debug')('talk:users'); router.get('/', authorization.needed('admin'), (req, res, next) => { const { @@ -104,6 +105,8 @@ router.post('/update-password', (req, res, next) => { router.post('/request-password-reset', (req, res, next) => { const {email} = req.body; + debug('/request-password-reset body %s', JSON.stringify(req.body, null, 2)); + if (!email) { return next('you must submit an email when requesting a password.'); } @@ -112,6 +115,7 @@ router.post('/request-password-reset', (req, res, next) => { .createPasswordResetToken(email) .then(token => { if (token === null) { + debug('back in the route. the token was null for some reason %s', token); return Promise.resolve('the email was not found in the db.'); } @@ -125,10 +129,15 @@ router.post('/request-password-reset', (req, res, next) => { rootURL: process.env.TALK_ROOT_URL }) }; + + debug('about to send a simple email with the options %s', JSON.stringify(options, null, 2)); + return mailer.sendSimple(options); }) .then(() => { + debug('password reset email sent successfully'); + // we want to send a 204 regardless of the user being found in the db // if we fail on missing emails, it would reveal if people are registered or not. res.status(204).end(); @@ -136,6 +145,8 @@ router.post('/request-password-reset', (req, res, next) => { .catch(error => { const errorMsg = typeof error === 'string' ? error : error.message; + debug('there was an error sending your email %s', error); + res.status(500).json({error: errorMsg}); }); }); From d2ba04def1f278d459ed32537ebeb63a39062b14 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 10:28:58 -1000 Subject: [PATCH 07/14] add a comma --- models/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/user.js b/models/user.js index 8c478fbb4..cabab8b64 100644 --- a/models/user.js +++ b/models/user.js @@ -455,7 +455,7 @@ UserService.createPasswordResetToken = function (email) { return Promise.resolve(null); } - debug('found the user! %s going to create the token', JSON.stringify(user, null 2)); + debug('found the user! %s going to create the token', JSON.stringify(user, null, 2)); const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; From 1c2b2c142ec5878e1f6c6a0b6178c8d668791592 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 11:04:57 -1000 Subject: [PATCH 08/14] change the environment vars. can't use sendgrid connection string --- .gitignore | 1 + services/mailer.js | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 5a42acb67..c76651868 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ dist/coral-admin/bundle.js tests/e2e/reports .DS_Store *.iml +*.swp dump.rdb .env gaba.cfg diff --git a/services/mailer.js b/services/mailer.js index 4da8024ca..0e9accf2b 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,10 +1,17 @@ const nodemailer = require('nodemailer'); -if (!process.env.TALK_SMTP_CONNECTION_URL) { +if (!process.env.TALK_SMTP_USERNAME || !process.env.TALK_SMTP_PASSWORD) { console.error('TALK_SMTP_CONNECTION_URL should be defined if you would like to send password reset emails from Talk'); } -const defaultTransporter = nodemailer.createTransport(process.env.TALK_SMTP_CONNECTION_URL); +const defaultTransporter = nodemailer.createTransport({ + // https://github.com/nodemailer/nodemailer-wellknown#supported-services + service: 'SendGrid', + auth: { + user: process.env.TALK_SMTP_USERNAME, + pass: process.env.TALK_SMTP_PASSWORD + } +}); const mailer = { From 7012684c2371de45504d3b50dfaa7a94bef6f299 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 11:06:09 -1000 Subject: [PATCH 09/14] correct error logs --- services/mailer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/mailer.js b/services/mailer.js index 0e9accf2b..16da98f41 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,7 +1,7 @@ const nodemailer = require('nodemailer'); if (!process.env.TALK_SMTP_USERNAME || !process.env.TALK_SMTP_PASSWORD) { - console.error('TALK_SMTP_CONNECTION_URL should be defined if you would like to send password reset emails from Talk'); + console.error('TALK_SMTP_USERNAME and TALK_SMTP_PASSWORD should be defined if you would like to send password reset emails from Talk'); } const defaultTransporter = nodemailer.createTransport({ From 25cc1ecb20992a4c31fc3c99cb81e7eaa69ff40f Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 11:23:59 -1000 Subject: [PATCH 10/14] remove debug statements --- models/user.js | 13 ------------- routes/api/user/index.js | 12 ------------ 2 files changed, 25 deletions(-) diff --git a/models/user.js b/models/user.js index cabab8b64..336486a54 100644 --- a/models/user.js +++ b/models/user.js @@ -2,7 +2,6 @@ const mongoose = require('../mongoose'); const uuid = require('uuid'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); -const debug = require('debug')('talk:users'); // SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run // through during the salting process. @@ -436,35 +435,23 @@ UserService.findPublicByIdArray = (ids) => { */ UserService.createPasswordResetToken = function (email) { if (!email || typeof email !== 'string') { - debug('the email was missing to createPasswordResetToken. you sent %s', email); return Promise.reject('email is required when creating a JWT for resetting passord'); } - email = email.toLowerCase(); - debug('about to look up user by email %s', email); - return UserModel.findOne({profiles: {$elemMatch: {id: email}}}) .then(user => { if (user === null) { - debug('user was null on createPasswordResetToken lookup'); // since we don't want to reveal that the email does/doesn't exist // just go ahead and resolve the Promise with null and check in the endpoint return Promise.resolve(null); } - debug('found the user! %s going to create the token', JSON.stringify(user, null, 2)); - const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; - - debug('created payload %s', JSON.stringify(payload)); - const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); - debug('successfully created the token %s', token); - return token; }); }; diff --git a/routes/api/user/index.js b/routes/api/user/index.js index 05ebf7b61..fa78b7dfe 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -8,7 +8,6 @@ const path = require('path'); const resetEmailFile = fs.readFileSync(path.resolve(__dirname, '../../../views/password-reset-email.ejs')); const resetEmailTemplate = ejs.compile(resetEmailFile.toString()); const authorization = require('../../../middleware/authorization'); -const debug = require('debug')('talk:users'); router.get('/', authorization.needed('admin'), (req, res, next) => { const { @@ -105,8 +104,6 @@ router.post('/update-password', (req, res, next) => { router.post('/request-password-reset', (req, res, next) => { const {email} = req.body; - debug('/request-password-reset body %s', JSON.stringify(req.body, null, 2)); - if (!email) { return next('you must submit an email when requesting a password.'); } @@ -115,7 +112,6 @@ router.post('/request-password-reset', (req, res, next) => { .createPasswordResetToken(email) .then(token => { if (token === null) { - debug('back in the route. the token was null for some reason %s', token); return Promise.resolve('the email was not found in the db.'); } @@ -130,23 +126,15 @@ router.post('/request-password-reset', (req, res, next) => { }) }; - debug('about to send a simple email with the options %s', JSON.stringify(options, null, 2)); - return mailer.sendSimple(options); }) .then(() => { - - debug('password reset email sent successfully'); - // we want to send a 204 regardless of the user being found in the db // if we fail on missing emails, it would reveal if people are registered or not. res.status(204).end(); }) .catch(error => { const errorMsg = typeof error === 'string' ? error : error.message; - - debug('there was an error sending your email %s', error); - res.status(500).json({error: errorMsg}); }); }); From a4af29cfa08912c072ca3f158b1b5188923a76bf Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 11:32:42 -1000 Subject: [PATCH 11/14] put things back --- routes/api/user/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routes/api/user/index.js b/routes/api/user/index.js index fa78b7dfe..1765809ad 100644 --- a/routes/api/user/index.js +++ b/routes/api/user/index.js @@ -125,16 +125,17 @@ router.post('/request-password-reset', (req, res, next) => { rootURL: process.env.TALK_ROOT_URL }) }; - return mailer.sendSimple(options); }) .then(() => { + // we want to send a 204 regardless of the user being found in the db // if we fail on missing emails, it would reveal if people are registered or not. res.status(204).end(); }) .catch(error => { const errorMsg = typeof error === 'string' ? error : error.message; + res.status(500).json({error: errorMsg}); }); }); From 5d4abf7269265dcb4a3e301ff8765b410762e6ea Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 14:55:12 -1000 Subject: [PATCH 12/14] add host and port because GCP is silly --- services/mailer.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/services/mailer.js b/services/mailer.js index 16da98f41..c21e0137a 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,12 +1,22 @@ const nodemailer = require('nodemailer'); -if (!process.env.TALK_SMTP_USERNAME || !process.env.TALK_SMTP_PASSWORD) { - console.error('TALK_SMTP_USERNAME and TALK_SMTP_PASSWORD should be defined if you would like to send password reset emails from Talk'); -} +const smtpProps = [ + 'TALK_SMTP_USERNAME', + 'TALK_SMTP_PASSWORD', + 'TALK_SMTP_HOST', + 'TALK_SMTP_PORT' +]; + +smtpProps.forEach(prop => { + if (!process.env[prop]) { + console.error(`process.env.${prop} should be defined if you would like to send password reset emails from Talk`); + } +}); const defaultTransporter = nodemailer.createTransport({ // https://github.com/nodemailer/nodemailer-wellknown#supported-services - service: 'SendGrid', + host: process.env.TALK_SMTP_HOST, + port: process.env.TALK_SMTP_PORT, auth: { user: process.env.TALK_SMTP_USERNAME, pass: process.env.TALK_SMTP_PASSWORD From f7fa0ae86644f745b6c0be68f9314dbcf6e2bc02 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 15:01:07 -1000 Subject: [PATCH 13/14] make PORT and HOST optional --- services/mailer.js | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/services/mailer.js b/services/mailer.js index c21e0137a..ed636314b 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,27 +1,34 @@ const nodemailer = require('nodemailer'); -const smtpProps = [ +const smtpRequiredProps = [ 'TALK_SMTP_USERNAME', - 'TALK_SMTP_PASSWORD', - 'TALK_SMTP_HOST', - 'TALK_SMTP_PORT' + 'TALK_SMTP_PASSWORD' ]; -smtpProps.forEach(prop => { +smtpRequiredProps.forEach(prop => { if (!process.env[prop]) { console.error(`process.env.${prop} should be defined if you would like to send password reset emails from Talk`); } }); -const defaultTransporter = nodemailer.createTransport({ - // https://github.com/nodemailer/nodemailer-wellknown#supported-services - host: process.env.TALK_SMTP_HOST, - port: process.env.TALK_SMTP_PORT, +// https://github.com/nodemailer/nodemailer-wellknown#supported-services +const options = { + service: 'SendGrid', auth: { user: process.env.TALK_SMTP_USERNAME, pass: process.env.TALK_SMTP_PASSWORD } -}); +}; + +if (process.env.TALK_SMTP_PORT) { + options.port = process.env.TALK_SMTP_PORT; +} + +if (process.env.TALK_SMTP_HOST) { + options.host = process.env.TALK_SMTP_HOST; +} + +const defaultTransporter = nodemailer.createTransport(options); const mailer = { From 9f360cf7800c57df07cce8b61d2d6151550a31a1 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 7 Dec 2016 15:03:49 -1000 Subject: [PATCH 14/14] make the provider an env var --- services/mailer.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/services/mailer.js b/services/mailer.js index ed636314b..b7d621954 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -2,7 +2,8 @@ const nodemailer = require('nodemailer'); const smtpRequiredProps = [ 'TALK_SMTP_USERNAME', - 'TALK_SMTP_PASSWORD' + 'TALK_SMTP_PASSWORD', + 'TALK_SMTP_PROVIDER' ]; smtpRequiredProps.forEach(prop => { @@ -11,9 +12,10 @@ smtpRequiredProps.forEach(prop => { } }); -// https://github.com/nodemailer/nodemailer-wellknown#supported-services const options = { - service: 'SendGrid', + // list of providers here: + // https://github.com/nodemailer/nodemailer-wellknown#supported-services + service: process.env.TALK_SMTP_PROVIDER, auth: { user: process.env.TALK_SMTP_USERNAME, pass: process.env.TALK_SMTP_PASSWORD