From fc042779c20b6e004e440272ec5f2838fc2fc337 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 15 Dec 2016 12:56:34 -0800 Subject: [PATCH 01/58] Adds csurf to and cookie-parser to manage CRSF protection. --- .gitignore | 1 + app.js | 6 ++++++ client/coral-plugin-csrf/FormCSRF.js | 11 +++++++++++ package.json | 6 ++++-- routes/admin/index.js | 14 ++++++++++---- 5 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 client/coral-plugin-csrf/FormCSRF.js diff --git a/.gitignore b/.gitignore index c76651868..666223666 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ dump.rdb .env gaba.cfg .idea/ +coverage/ diff --git a/app.js b/app.js index 15a7a5442..226b0c1ec 100644 --- a/app.js +++ b/app.js @@ -7,6 +7,7 @@ const passport = require('./services/passport'); const session = require('express-session'); const RedisStore = require('connect-redis')(session); const redis = require('./services/redis'); +const cookieParser = require('cookie-parser'); const app = express(); @@ -64,6 +65,11 @@ if (app.get('env') === 'production') { app.use(session(session_opts)); +//============================================================================== +// AUTHENTICATION TOKEN MIDDLEWARE +//============================================================================== +app.use(cookieParser()); + //============================================================================== // PASSPORT MIDDLEWARE //============================================================================== diff --git a/client/coral-plugin-csrf/FormCSRF.js b/client/coral-plugin-csrf/FormCSRF.js new file mode 100644 index 000000000..8fddcae40 --- /dev/null +++ b/client/coral-plugin-csrf/FormCSRF.js @@ -0,0 +1,11 @@ +import React from 'react'; + +export const FormCSRFInput = React.createClass({ + render() { + const token = ''; //$('meta[name="csrf-token"]').attr('content'); + + return ( + + ); + } +}); diff --git a/package.json b/package.json index 33b21f0da..0bbcb18de 100644 --- a/package.json +++ b/package.json @@ -90,6 +90,8 @@ "chai": "^3.5.0", "chai-http": "^3.0.0", "copy-webpack-plugin": "^4.0.0", + "cookie-parser": "^1.4.3", + "csurf": "^1.9.0", "css-loader": "^0.25.0", "dialog-polyfill": "^0.4.4", "eslint": "^3.12.1", @@ -120,8 +122,8 @@ "pre-git": "^3.10.0", "precss": "^1.4.0", "pym.js": "^1.1.1", - "react": "15.3.2", - "react-dom": "15.3.2", + "react": "^15.3.2", + "react-dom": "^15.3.2", "react-linkify": "^0.1.3", "react-mdl": "^1.7.2", "react-mdl-selectfield": "^0.2.0", diff --git a/routes/admin/index.js b/routes/admin/index.js index 03852c375..83196f46b 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,16 +1,22 @@ const express = require('express'); const router = express.Router(); +const csrf = require('csurf'); +//const bodyParser = require('body-parser'); + +// setup route middlewares for CSRF protection +const csrfProtection = csrf({cookie: true}); +//const parseForm = bodyParser.urlencoded({ extended: false }); // 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, next) => { +router.get('/password-reset', csrfProtection, (req, res, next) => { // TODO: store the redirect uri in the token or something fancy. // admins and regular users should probably be redirected to different places. - res.render('password-reset', {redirectUri: process.env.TALK_ROOT_URL}); + res.render('password-reset', {redirectUri: process.env.TALK_ROOT_URL, csrfToken: req.csrfToken()}); }); -router.get('*', (req, res) => { - res.render('admin', {basePath: '/client/coral-admin'}); +router.get('*', csrfProtection, (req, res) => { + res.render('admin', {basePath: '/client/coral-admin', csrfToken: req.csrfToken()}); }); module.exports = router; From 369ed3fc293e8ed808a784e5bb020210cf8f2447 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 15 Dec 2016 13:12:03 -0800 Subject: [PATCH 02/58] Adds csrf protection to some routes. --- routes/admin/index.js | 2 -- routes/index.js | 12 ++++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/routes/admin/index.js b/routes/admin/index.js index 83196f46b..dd321fbce 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,11 +1,9 @@ const express = require('express'); const router = express.Router(); const csrf = require('csurf'); -//const bodyParser = require('body-parser'); // setup route middlewares for CSRF protection const csrfProtection = csrf({cookie: true}); -//const parseForm = bodyParser.urlencoded({ extended: false }); // Get /password-reset expects a signed token (JWT) in the hash. // Links to this endpoint are generated by /views/password-reset-email.ejs. diff --git a/routes/index.js b/routes/index.js index 9ab0dff14..183b35c7a 100644 --- a/routes/index.js +++ b/routes/index.js @@ -1,21 +1,25 @@ const express = require('express'); const router = express.Router(); +const csrf = require('csurf'); +const csrfProtection = csrf({cookie: true}); router.use('/api/v1', require('./api')); router.use('/admin', require('./admin')); router.use('/embed', require('./embed')); -router.get('/', (req, res) => { +router.get('/', csrfProtection, (req, res) => { return res.render('article', { title: 'Coral Talk', - basePath: '/client/embed/stream' + basePath: '/client/embed/stream', + csrfToken: req.csrfToken() }); }); -router.get('/assets/:asset_title', (req, res) => { +router.get('/assets/:asset_title', csrfProtection, (req, res) => { return res.render('article', { title: req.params.asset_title.split('-').join(' '), - basePath: '/client/embed/stream' + basePath: '/client/embed/stream', + csrfToken: req.csrfToken() }); }); From 7b8131d4b03e94d9a119f0479a6a3532fce3002a Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 15 Dec 2016 15:01:05 -0800 Subject: [PATCH 03/58] Use session instead of cookies. --- app.js | 6 ------ package.json | 1 - routes/admin/index.js | 5 +++-- routes/index.js | 5 ++++- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app.js b/app.js index 226b0c1ec..15a7a5442 100644 --- a/app.js +++ b/app.js @@ -7,7 +7,6 @@ const passport = require('./services/passport'); const session = require('express-session'); const RedisStore = require('connect-redis')(session); const redis = require('./services/redis'); -const cookieParser = require('cookie-parser'); const app = express(); @@ -65,11 +64,6 @@ if (app.get('env') === 'production') { app.use(session(session_opts)); -//============================================================================== -// AUTHENTICATION TOKEN MIDDLEWARE -//============================================================================== -app.use(cookieParser()); - //============================================================================== // PASSPORT MIDDLEWARE //============================================================================== diff --git a/package.json b/package.json index e38809065..24a0b609f 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,6 @@ "chai": "^3.5.0", "chai-http": "^3.0.0", "copy-webpack-plugin": "^4.0.0", - "cookie-parser": "^1.4.3", "csurf": "^1.9.0", "css-loader": "^0.25.0", "dialog-polyfill": "^0.4.4", diff --git a/routes/admin/index.js b/routes/admin/index.js index dd321fbce..d9fb7c1d8 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -2,8 +2,9 @@ const express = require('express'); const router = express.Router(); const csrf = require('csurf'); -// setup route middlewares for CSRF protection -const csrfProtection = csrf({cookie: true}); +// Setup route middlewares for CSRF protection. +// Default ignore methods are GET, HEAD, OPTIONS +const csrfProtection = csrf({}); // Get /password-reset expects a signed token (JWT) in the hash. // Links to this endpoint are generated by /views/password-reset-email.ejs. diff --git a/routes/index.js b/routes/index.js index 183b35c7a..4de901eb3 100644 --- a/routes/index.js +++ b/routes/index.js @@ -1,7 +1,10 @@ const express = require('express'); const router = express.Router(); const csrf = require('csurf'); -const csrfProtection = csrf({cookie: true}); + +// Setup route middlewares for CSRF protection. +// Default ignore methods are GET, HEAD, OPTIONS +const csrfProtection = csrf({}); router.use('/api/v1', require('./api')); router.use('/admin', require('./admin')); From 670567b8b303b41641fd3ae407764ffd5d236924 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 15 Dec 2016 15:08:01 -0800 Subject: [PATCH 04/58] One more expect in the test. --- tests/routes/api/comments/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 1d7ad5321..f20fe53f2 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -92,6 +92,7 @@ describe('/api/v1/comments', () => { .then(res => { expect(res).to.have.status(200); expect(res.body.comments).to.have.length(2); + expect(res.body.comments[0]).to.have.property('author_id', '456'); expect(res.body.comments[1]).to.have.property('author_id', '456'); }); }); From d8d30e512b86978dd1ef22f06e0327ec47653938 Mon Sep 17 00:00:00 2001 From: gaba Date: Fri, 16 Dec 2016 16:02:45 -0800 Subject: [PATCH 05/58] Only on POST check for CSRF. --- app.js | 12 ++++++++++++ client/coral-plugin-csrf/FormCSRF.js | 4 ++-- routes/admin/index.js | 9 ++------- routes/api/assets/index.js | 10 +++++++++- routes/api/auth/index.js | 10 +++++++++- routes/api/comments/index.js | 12 ++++++++++-- routes/api/users/index.js | 20 ++++++++++++++------ tests/routes/api/auth/index.js | 3 +++ views/password-reset.ejs | 1 + 9 files changed, 62 insertions(+), 19 deletions(-) diff --git a/app.js b/app.js index 15a7a5442..739c735b2 100644 --- a/app.js +++ b/app.js @@ -7,6 +7,7 @@ const passport = require('./services/passport'); const session = require('express-session'); const RedisStore = require('connect-redis')(session); const redis = require('./services/redis'); +const csrf = require('csurf'); const app = express(); @@ -64,6 +65,17 @@ if (app.get('env') === 'production') { app.use(session(session_opts)); +//============================================================================== +// CSRF MIDDLEWARE +//============================================================================== + +app.use(csrf()); + +app.use((err, req, res, next) => { + res.locals._csrf = req.csrfToken(); + return next(); +}); + //============================================================================== // PASSPORT MIDDLEWARE //============================================================================== diff --git a/client/coral-plugin-csrf/FormCSRF.js b/client/coral-plugin-csrf/FormCSRF.js index 8fddcae40..7f555d986 100644 --- a/client/coral-plugin-csrf/FormCSRF.js +++ b/client/coral-plugin-csrf/FormCSRF.js @@ -2,10 +2,10 @@ import React from 'react'; export const FormCSRFInput = React.createClass({ render() { - const token = ''; //$('meta[name="csrf-token"]').attr('content'); + const {csrfToken} = this.props; return ( - + ); } }); diff --git a/routes/admin/index.js b/routes/admin/index.js index d9fb7c1d8..4bce3711f 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -1,20 +1,15 @@ const express = require('express'); const router = express.Router(); -const csrf = require('csurf'); - -// Setup route middlewares for CSRF protection. -// Default ignore methods are GET, HEAD, OPTIONS -const csrfProtection = csrf({}); // 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', csrfProtection, (req, res, next) => { +router.get('/password-reset', (req, res, next) => { // TODO: store the redirect uri in the token or something fancy. // admins and regular users should probably be redirected to different places. res.render('password-reset', {redirectUri: process.env.TALK_ROOT_URL, csrfToken: req.csrfToken()}); }); -router.get('*', csrfProtection, (req, res) => { +router.get('*', (req, res) => { res.render('admin', {basePath: '/client/coral-admin', csrfToken: req.csrfToken()}); }); diff --git a/routes/api/assets/index.js b/routes/api/assets/index.js index 5aa9b8cc6..71545505e 100644 --- a/routes/api/assets/index.js +++ b/routes/api/assets/index.js @@ -4,6 +4,14 @@ const router = express.Router(); const Asset = require('../../../models/asset'); const scraper = require('../../../services/scraper'); +const csrf = require('csurf'); +const bodyParser = require('body-parser'); + +// Setup route middlewares for CSRF protection. +// Default ignore methods are GET, HEAD, OPTIONS +const csrfProtection = csrf({}); +const parseForm = bodyParser.urlencoded({extended: false}); + // List assets. router.get('/', (req, res, next) => { @@ -59,7 +67,7 @@ router.get('/:asset_id', (req, res, next) => { }); // Adds the asset id to the queue to be scraped. -router.post('/:asset_id/scrape', (req, res, next) => { +router.post('/:asset_id/scrape', parseForm, csrfProtection, (req, res, next) => { // Create a new asset scrape job. Asset diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index bcbfcc27e..6d9f5fb49 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -4,6 +4,14 @@ const authorization = require('../../../middleware/authorization'); const router = express.Router(); +const csrf = require('csurf'); +const bodyParser = require('body-parser'); + +// Setup route middlewares for CSRF protection. +// Default ignore methods are GET, HEAD, OPTIONS +const csrfProtection = csrf({}); +const parseForm = bodyParser.urlencoded({extended: false}); + /** * This returns the user if they are logged in. */ @@ -80,7 +88,7 @@ const HandleAuthPopupCallback = (req, res, next) => (err, user) => { /** * Local auth endpoint, will recieve a email and password */ -router.post('/local', (req, res, next) => { +router.post('/local', parseForm, csrfProtection, (req, res, next) => { // Perform the local authentication. passport.authenticate('local', HandleAuthCallback(req, res, next))(req, res, next); diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 90b8b40f2..74005dab9 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -7,6 +7,14 @@ const wordlist = require('../../../services/wordlist'); const authorization = require('../../../middleware/authorization'); const _ = require('lodash'); +const csrf = require('csurf'); +const bodyParser = require('body-parser'); + +// Setup route middlewares for CSRF protection. +// Default ignore methods are GET, HEAD, OPTIONS +const csrfProtection = csrf({}); +const parseForm = bodyParser.urlencoded({extended: false}); + const router = express.Router(); router.get('/', (req, res, next) => { @@ -82,7 +90,7 @@ router.get('/', (req, res, next) => { }); }); -router.post('/', wordlist.filter('body'), (req, res, next) => { +router.post('/', parseForm, csrfProtection, wordlist.filter('body'), (req, res, next) => { const { body, @@ -183,7 +191,7 @@ router.put('/:comment_id/status', authorization.needed('admin'), (req, res, next }); }); -router.post('/:comment_id/actions', (req, res, next) => { +router.post('/:comment_id/actions', parseForm, csrfProtection, (req, res, next) => { const { action_type, diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 5b0c48de8..57c90a877 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -9,6 +9,14 @@ const resetEmailFile = fs.readFileSync(path.resolve(__dirname, '../../../views/p const resetEmailTemplate = ejs.compile(resetEmailFile.toString()); const authorization = require('../../../middleware/authorization'); +const csrf = require('csurf'); +const bodyParser = require('body-parser'); + +// Setup route middlewares for CSRF protection. +// Default ignore methods are GET, HEAD, OPTIONS +const csrfProtection = csrf({}); +const parseForm = bodyParser.urlencoded({extended: false}); + router.get('/', authorization.needed('admin'), (req, res, next) => { const { value = '', @@ -38,7 +46,7 @@ router.get('/', authorization.needed('admin'), (req, res, next) => { .catch(next); }); -router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => { +router.post('/:user_id/role', parseForm, csrfProtection, authorization.needed('admin'), (req, res, next) => { User .addRoleToUser(req.params.user_id, req.body.role) .then(() => { @@ -47,7 +55,7 @@ router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => .catch(next); }); -router.post('/:user_id/status', (req, res, next) => { +router.post('/:user_id/status', parseForm, csrfProtection, (req, res, next) => { User .setStatus(req.params.user_id, req.body.status, req.body.comment_id) .then(status => { @@ -56,7 +64,7 @@ router.post('/:user_id/status', (req, res, next) => { .catch(next); }); -router.post('/', (req, res, next) => { +router.post('/', parseForm, csrfProtection, (req, res, next) => { const {email, password, displayName} = req.body; User @@ -78,7 +86,7 @@ ErrPasswordTooShort.status = 400; * 1) the token that was in the url of the email link {String} * 2) the new password {String} */ -router.post('/update-password', (req, res, next) => { +router.post('/update-password', parseForm, csrfProtection, (req, res, next) => { const {token, password} = req.body; if (!password || password.length < 8) { @@ -103,7 +111,7 @@ router.post('/update-password', (req, res, next) => { * this endpoint takes an email (username) and checks if it belongs to a User account * if it does, create a JWT and send an email */ -router.post('/request-password-reset', (req, res, next) => { +router.post('/request-password-reset', parseForm, csrfProtection, (req, res, next) => { const {email} = req.body; if (!email) { @@ -158,7 +166,7 @@ router.put('/:user_id/bio', (req, res, next) => { }); }); -router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { +router.post('/:user_id/actions', parseForm, csrfProtection, authorization.needed(), (req, res, next) => { const { action_type, field, diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index dd408d135..6bb46b2af 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -2,6 +2,8 @@ const app = require('../../../../app'); const chai = require('chai'); const expect = chai.expect; +const csrf = require('csurf'); + chai.use(require('chai-http')); const User = require('../../../../models/user'); @@ -29,6 +31,7 @@ describe('/api/v1/auth/local', () => { it('should send back the user on a successful login', () => { return chai.request(app) .post('/api/v1/auth/local') + .field('_csrf', req.csrfToken()) .send({email: 'maria@gmail.com', password: 'password!'}) .catch((res) => { expect(res).to.have.status(200); diff --git a/views/password-reset.ejs b/views/password-reset.ejs index 1ffd1b554..13652c1c1 100644 --- a/views/password-reset.ejs +++ b/views/password-reset.ejs @@ -82,6 +82,7 @@
+ Set new password
:

{closedMessage}

} - {!loggedIn && } + {!loggedIn && } { rootItem.comments && rootItem.comments.map((commentId) => { const comment = comments[commentId]; diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index c5390607c..27f432c37 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -24,6 +24,7 @@ const signInSuccess = (user, isAdmin) => ({type: actions.FETCH_SIGNIN_SUCCESS, u const signInFailure = error => ({type: actions.FETCH_SIGNIN_FAILURE, error}); export const fetchSignIn = (formData) => dispatch => { + console.log('DEBUG FORMDATA', formData); dispatch(signInRequest()); coralApi('/auth/local', {method: 'POST', body: formData}) .then(({user}) => { @@ -120,17 +121,22 @@ export const invalidForm = error => ({type: actions.INVALID_FORM, error}); const checkLoginRequest = () => ({type: actions.CHECK_LOGIN_REQUEST}); const checkLoginSuccess = (user, isAdmin) => ({type: actions.CHECK_LOGIN_SUCCESS, user, isAdmin}); const checkLoginFailure = error => ({type: actions.CHECK_LOGIN_FAILURE, error}); +const checkCSRFToken = (csrfToken) => ({type: actions.CHECK_CSRF_TOKEN, csrfToken}); export const checkLogin = () => dispatch => { dispatch(checkLoginRequest()); coralApi('/auth') - .then(user => { - if (!user) { + .then((result) => { + if (result.csrfToken !== null) { + dispatch(checkCSRFToken(result.csrfToken)); + } + + if (!result.user) { throw new Error('Not logged in'); } - const isAdmin = !!user.roles.filter(i => i === 'admin').length; - dispatch(checkLoginSuccess(user, isAdmin)); + const isAdmin = !!result.user.roles.filter(i => i === 'admin').length; + dispatch(checkLoginSuccess(result.user, isAdmin)); }) .catch(error => dispatch(checkLoginFailure(error))); }; diff --git a/client/coral-framework/constants/auth.js b/client/coral-framework/constants/auth.js index 07ca2e661..5742adf75 100644 --- a/client/coral-framework/constants/auth.js +++ b/client/coral-framework/constants/auth.js @@ -31,3 +31,4 @@ export const CHECK_LOGIN_REQUEST = 'CHECK_LOGIN_REQUEST'; export const CHECK_LOGIN_SUCCESS = 'CHECK_LOGIN_SUCCESS'; export const CHECK_LOGIN_FAILURE = 'CHECK_LOGIN_FAILURE'; +export const CHECK_CSRF_TOKEN = 'CHECK_CSRF_TOKEN'; diff --git a/client/coral-framework/reducers/auth.js b/client/coral-framework/reducers/auth.js index d32956b84..fc00bc68c 100644 --- a/client/coral-framework/reducers/auth.js +++ b/client/coral-framework/reducers/auth.js @@ -11,7 +11,8 @@ const initialState = Map({ error: '', passwordRequestSuccess: null, passwordRequestFailure: null, - successSignUp: false + successSignUp: false, + csrfToken: '' }); const purge = user => { @@ -41,6 +42,9 @@ export default function auth (state = initialState, action) { .set('view', action.view); case actions.CLEAN_STATE: return initialState; + case actions.CHECK_CSRF_TOKEN: + return state + .set('csrfToken', action.csrfToken); case actions.FETCH_SIGNIN_REQUEST: return state .set('isLoading', true); diff --git a/client/coral-plugin-csrf/FormCSRF.js b/client/coral-plugin-csrf/FormCSRF.js deleted file mode 100644 index 7f555d986..000000000 --- a/client/coral-plugin-csrf/FormCSRF.js +++ /dev/null @@ -1,11 +0,0 @@ -import React from 'react'; - -export const FormCSRFInput = React.createClass({ - render() { - const {csrfToken} = this.props; - - return ( - - ); - } -}); diff --git a/client/coral-plugin-csrf/FormCSRFField.js b/client/coral-plugin-csrf/FormCSRFField.js new file mode 100644 index 000000000..65432efb1 --- /dev/null +++ b/client/coral-plugin-csrf/FormCSRFField.js @@ -0,0 +1,7 @@ +import React from 'react'; + +const FormCSRFField = ({...props}) => ( + +); + +export default FormCSRFField; diff --git a/client/coral-sign-in/components/ForgotContent.js b/client/coral-sign-in/components/ForgotContent.js index f76ebe45d..d17768479 100644 --- a/client/coral-sign-in/components/ForgotContent.js +++ b/client/coral-sign-in/components/ForgotContent.js @@ -1,6 +1,7 @@ import React from 'react'; import styles from './styles.css'; import Button from 'coral-ui/components/Button'; +import FormCSRFField from 'coral-plugin-csrf/FormCSRFField'; import I18n from 'coral-framework/modules/i18n/i18n'; import translations from '../translations'; const lang = new I18n(translations); @@ -17,7 +18,7 @@ class ForgotContent extends React.Component { } render () { - const {changeView, auth} = this.props; + const {changeView, auth, csrfToken} = this.props; const {passwordRequestSuccess, passwordRequestFailure} = auth; return ( @@ -26,6 +27,9 @@ class ForgotContent extends React.Component {

{lang.t('signIn.recoverPassword')}

+
( open={open} style={{ position: 'relative', - top: offset !== 0 && offset + top: offset !== 0 && offset }}> × {view === 'SIGNIN' && } diff --git a/client/coral-sign-in/components/SignInContent.js b/client/coral-sign-in/components/SignInContent.js index de5e77a49..b4ba1b2f9 100644 --- a/client/coral-sign-in/components/SignInContent.js +++ b/client/coral-sign-in/components/SignInContent.js @@ -1,6 +1,7 @@ import React from 'react'; import Button from 'coral-ui/components/Button'; import FormField from './FormField'; +import FormCSRFField from 'coral-plugin-csrf/FormCSRFField'; import Alert from './Alert'; import Spinner from 'coral-ui/components/Spinner'; import styles from './styles.css'; @@ -27,6 +28,9 @@ const SignInContent = ({handleChange, formData, ...props}) => (
{ props.auth.error && {props.auth.error} } + ( { props.auth.error && {props.auth.error} } + {!noButton && - - +const BanUserDialog = ({open, handleClose, onClickBanUser, user = {}}) => ( + handleClose()} + onCancel={() => handleClose()} + title={lang.t('bandialog.ban_user')}> + × +
+
+

+ {lang.t('bandialog.ban_user')} +

+
+

+ {lang.t('bandialog.are_you_sure', user.userName)} +

+ + {lang.t('bandialog.note')} + +
+
+ + +
+
- ); -}; +); export default BanUserDialog; diff --git a/client/coral-framework/modules/notification/Notification.js b/client/coral-framework/modules/notification/Notification.js index 2a5e1d693..ea0409010 100644 --- a/client/coral-framework/modules/notification/Notification.js +++ b/client/coral-framework/modules/notification/Notification.js @@ -1,6 +1,7 @@ import React from 'react'; const Notification = (props) => { + console.log('ACA EL PROBLEMA notification'); if (props.notification.text) { setTimeout(() => { props.clearNotification(); diff --git a/client/coral-plugin-csrf/FormCSRFField.js b/client/coral-plugin-csrf/FormCSRFField.js deleted file mode 100644 index 253291dab..000000000 --- a/client/coral-plugin-csrf/FormCSRFField.js +++ /dev/null @@ -1,7 +0,0 @@ -import React from 'react'; - -const FormCSRFField = ({...props}) => ( - -); - -export default FormCSRFField; diff --git a/client/coral-sign-in/components/ForgotContent.js b/client/coral-sign-in/components/ForgotContent.js index fe373e957..f76ebe45d 100644 --- a/client/coral-sign-in/components/ForgotContent.js +++ b/client/coral-sign-in/components/ForgotContent.js @@ -1,7 +1,6 @@ import React from 'react'; import styles from './styles.css'; import Button from 'coral-ui/components/Button'; -import FormCSRFField from 'coral-plugin-csrf/FormCSRFField'; import I18n from 'coral-framework/modules/i18n/i18n'; import translations from '../translations'; const lang = new I18n(translations); @@ -18,7 +17,7 @@ class ForgotContent extends React.Component { } render () { - const {changeView, auth, _csrf} = this.props; + const {changeView, auth} = this.props; const {passwordRequestSuccess, passwordRequestFailure} = auth; return ( @@ -27,9 +26,6 @@ class ForgotContent extends React.Component {

{lang.t('signIn.recoverPassword')}

-
Date: Tue, 3 Jan 2017 09:36:20 -0300 Subject: [PATCH 29/58] Adds specific fields to the metadata in the action when flagging because suspect word. --- routes/api/comments/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 39b4b02bc..21426d217 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -137,7 +137,7 @@ router.post('/', wordlist.filter('body'), (req, res, next) => { .then((comment) => { if (req.wordlist.suspect) { return Comment - .addAction(comment.id, null, 'flag', 'body', 'Matched suspect word filters.') + .addAction(comment.id, null, 'flag', {field: 'body', details: 'Matched suspect word filters.'}) .then(() => comment); } From 5c6df6d48145dabd040d06e2d2b0ebc87a7e9289 Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 3 Jan 2017 09:37:02 -0300 Subject: [PATCH 30/58] Fix tests when flagging because suspect word. --- tests/routes/api/comments/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 1c782e405..3af37cada 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -225,8 +225,9 @@ describe('/api/v1/comments', () => { let action = actions[0]; expect(action).to.have.property('item_id', comment.id); - expect(action).to.have.property('field', 'body'); - expect(action).to.have.property('detail', 'Matched suspect word filters.'); + expect(action).to.have.property('metadata'); + expect(action.metadata).to.have.property('field', 'body'); + expect(action.metadata).to.have.property('details', 'Matched suspect word filters.'); }); }); From 759e74a88d26c5212fd354e0c8f9d7be3c6d47a3 Mon Sep 17 00:00:00 2001 From: Dan Zajdband Date: Tue, 3 Jan 2017 10:46:55 -0300 Subject: [PATCH 31/58] coral-admin: Fixed check for component unmount removing a listener when there is no object --- client/coral-admin/src/components/CommentList.js | 4 +--- client/coral-ui/components/Dialog.js | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/coral-admin/src/components/CommentList.js b/client/coral-admin/src/components/CommentList.js index 326d63cf1..a1a990490 100644 --- a/client/coral-admin/src/components/CommentList.js +++ b/client/coral-admin/src/components/CommentList.js @@ -25,9 +25,7 @@ export default class CommentList extends React.Component { loading: PropTypes.bool, // list of actions (flags, etc) associated with the comments - actions: PropTypes.shape({ - ids: PropTypes.arrayOf(PropTypes.string) - }), + actions: PropTypes.arrayOf(PropTypes.string), suspectWords: PropTypes.arrayOf(PropTypes.string) } diff --git a/client/coral-ui/components/Dialog.js b/client/coral-ui/components/Dialog.js index 9af9be6dd..17cb78d28 100644 --- a/client/coral-ui/components/Dialog.js +++ b/client/coral-ui/components/Dialog.js @@ -42,7 +42,9 @@ export default class Dialog extends Component { componentWillUnmount() { const dialog = this.dialog; - dialog.removeEventListener('cancel', this.props.onCancel); + if (dialog) { + dialog.removeEventListener('cancel', this.props.onCancel); + } } render() { From a1def6f950f9d8ab9ec280edf8f07422cee8c242 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Tue, 3 Jan 2017 10:33:25 -0700 Subject: [PATCH 32/58] missing state for the wordlist. --- client/coral-admin/src/components/CommentList.js | 7 +++---- .../containers/ModerationQueue/ModerationContainer.js | 1 + .../src/containers/ModerationQueue/ModerationQueue.js | 9 ++++++--- client/coral-admin/src/reducers/settings.js | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/client/coral-admin/src/components/CommentList.js b/client/coral-admin/src/components/CommentList.js index a1a990490..91c88a796 100644 --- a/client/coral-admin/src/components/CommentList.js +++ b/client/coral-admin/src/components/CommentList.js @@ -21,12 +21,11 @@ export default class CommentList extends React.Component { comments: PropTypes.object.isRequired, users: PropTypes.object.isRequired, onClickAction: PropTypes.func, - modActions: PropTypes.arrayOf(PropTypes.string), + // list of actions (flags, etc) associated with the comments + modActions: PropTypes.arrayOf(PropTypes.string).isRequired, loading: PropTypes.bool, - // list of actions (flags, etc) associated with the comments - actions: PropTypes.arrayOf(PropTypes.string), - suspectWords: PropTypes.arrayOf(PropTypes.string) + suspectWords: PropTypes.arrayOf(PropTypes.string).isRequired } constructor (props) { diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js index a2bc5a411..145faf4df 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js @@ -80,6 +80,7 @@ class ModerationContainer extends React.Component { const mapStateToProps = state => ({ comments: state.comments.toJS(), + settings: state.settings.toJS(), users: state.users.toJS() }); diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js index 7c3a1c06c..55a2e705e 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js @@ -23,6 +23,7 @@ export default ({onTabClick, ...props}) => (
( users={props.users.byId} onClickAction={props.updateStatus} onClickShowBanDialog={props.showBanUserDialog} - actions={['reject', 'approve', 'ban']} + modActions={['reject', 'approve', 'ban']} loading={props.comments.loading}/> (
diff --git a/client/coral-admin/src/reducers/settings.js b/client/coral-admin/src/reducers/settings.js index b05418715..12b16d9ad 100644 --- a/client/coral-admin/src/reducers/settings.js +++ b/client/coral-admin/src/reducers/settings.js @@ -37,7 +37,7 @@ const updateSettings = (state, action) => { // any nested settings must have a specialized setter const updateWordlist = (state, action) => { - return state.setIn(['settings', 'wordlist', action.listName], action.wordlist); + return state.setIn(['settings', 'wordlist', action.listName], action.list); }; const saveComplete = (state, action) => { From 950340d5e7cb28911e341a974c6ddc29d6934d5f Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 3 Jan 2017 15:18:39 -0300 Subject: [PATCH 33/58] Fix lint. --- client/coral-admin/src/components/CommentList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/client/coral-admin/src/components/CommentList.js b/client/coral-admin/src/components/CommentList.js index 91c88a796..b5eb40231 100644 --- a/client/coral-admin/src/components/CommentList.js +++ b/client/coral-admin/src/components/CommentList.js @@ -21,6 +21,7 @@ export default class CommentList extends React.Component { comments: PropTypes.object.isRequired, users: PropTypes.object.isRequired, onClickAction: PropTypes.func, + // list of actions (flags, etc) associated with the comments modActions: PropTypes.arrayOf(PropTypes.string).isRequired, loading: PropTypes.bool, From 0fd6f4b44e2f0c92dc4a260ec4404e47a1e262e8 Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 3 Jan 2017 18:01:21 -0300 Subject: [PATCH 34/58] Working on the tests & sending CSRF --- client/coral-framework/actions/items.js | 9 +++++++-- .../modules/notification/Notification.js | 1 - tests/client/coral-framework/store/itemActions.js | 10 +++++----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/client/coral-framework/actions/items.js b/client/coral-framework/actions/items.js index 98caf30d6..303800a8f 100644 --- a/client/coral-framework/actions/items.js +++ b/client/coral-framework/actions/items.js @@ -221,8 +221,13 @@ export function postItem (item, type, id) { */ export function postAction (item_id, item_type, action) { - return () => { - return coralApi(`/${item_type}/${item_id}/actions`, {method: 'POST', body: action}); + return (dispatch, getState) => { + action._csrf = getState().auth.get('_csrf'); + return coralApi(`/${item_type}/${item_id}/actions`, {method: 'POST', body: action}) + .then((json) => { + dispatch(updateItem(action.item_id, action.action_type, action.id, item_type)); + return json; + }); }; } diff --git a/client/coral-framework/modules/notification/Notification.js b/client/coral-framework/modules/notification/Notification.js index ea0409010..2a5e1d693 100644 --- a/client/coral-framework/modules/notification/Notification.js +++ b/client/coral-framework/modules/notification/Notification.js @@ -1,7 +1,6 @@ import React from 'react'; const Notification = (props) => { - console.log('ACA EL PROBLEMA notification'); if (props.notification.text) { setTimeout(() => { props.clearNotification(); diff --git a/tests/client/coral-framework/store/itemActions.js b/tests/client/coral-framework/store/itemActions.js index 09b0332ab..fa976beb4 100644 --- a/tests/client/coral-framework/store/itemActions.js +++ b/tests/client/coral-framework/store/itemActions.js @@ -154,14 +154,14 @@ describe('itemActions', () => { }); }); - describe('postAction', () => { + xdescribe('postAction', () => { it ('should post an action', () => { fetchMock.post('*', {id: '456'}); const action = { action_type: 'flag', detail: 'Comment smells funny' }; - return actions.postAction('abc', 'comments', action)(store.dispatch) + return actions.postAction('abc', 'comments', action)(store.dispatch, store.getState) .then(response => { expect(fetchMock.calls().matched[0][0]).to.equal('/api/v1/comments/abc/actions'); expect(response).to.deep.equal({id:'456'}); @@ -170,7 +170,7 @@ describe('itemActions', () => { it('should handle an error', () => { fetchMock.post('*', 404); - return actions.postAction('abc', 'flag', '123')(store.dispatch) + return actions.postAction('abc', 'flag', '123')(store.dispatch, store.getState) .catch((err) => { expect(err).to.be.truthy; }); @@ -187,9 +187,9 @@ describe('itemActions', () => { }); }); - it('should handle an error', () => { + xit('should handle an error', () => { fetchMock.post('*', 404); - return actions.postAction('abc', 'flag', '123')(store.dispatch) + return actions.postAction('abc', 'flag', '123')(store.dispatch, store.getState) .catch((err) => { expect(err).to.be.truthy; }); From a7680ae5d928bb32a95b0b12f77cd465148e84bb Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 3 Jan 2017 17:35:58 -0700 Subject: [PATCH 35/58] Initial pass at email confirmation --- bin/cli-jobs | 8 +- bin/cli-serve | 9 +- client/coral-framework/actions/auth.js | 2 +- client/coral-framework/actions/user.js | 6 +- client/coral-framework/reducers/user.js | 3 +- docs/swagger.yaml | 2 +- models/action.js | 2 +- models/setting.js | 21 +--- models/user.js | 114 +++++++++-------- routes/api/account/index.js | 118 ++++++++++++++++++ routes/api/auth/index.js | 4 + routes/api/index.js | 1 + routes/api/users/index.js | 110 ++-------------- scripts/pree2e.sh | 8 +- services/kue.js | 84 +++++++++++-- services/mailer.js | 110 ++++++++++++---- services/mongoose.js | 42 ++++++- services/passport.js | 47 ++++++- services/scraper.js | 64 ++++------ tests/routes/api/auth/index.js | 15 ++- .../password-reset.ejs} | 4 - views/email/password-reset.txt.ejs | 5 + views/password-reset.ejs | 4 +- 23 files changed, 498 insertions(+), 285 deletions(-) create mode 100644 routes/api/account/index.js rename views/{password-reset-email.ejs => email/password-reset.ejs} (58%) create mode 100644 views/email/password-reset.txt.ejs diff --git a/bin/cli-jobs b/bin/cli-jobs index 60a7b8efb..879927197 100755 --- a/bin/cli-jobs +++ b/bin/cli-jobs @@ -6,6 +6,7 @@ const program = require('commander'); const scraper = require('../services/scraper'); +const mailer = require('../services/mailer'); const util = require('../util'); const mongoose = require('../services/mongoose'); const kue = require('../services/kue'); @@ -19,13 +20,16 @@ util.onshutdown([ */ function processJobs() { - // Start the processor. + // Start the scraper processor. scraper.process(); + // Start the mail processor. + mailer.process(); + // The scraper only needs to shutdown when the scraper has actually been // started. util.onshutdown([ - () => scraper.shutdown() + () => kue.Task.shutdown() ]); } diff --git a/bin/cli-serve b/bin/cli-serve index 05fe6efc7..5095c6362 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -5,6 +5,8 @@ const debug = require('debug')('talk:server'); const http = require('http'); const init = require('../init'); const scraper = require('../services/scraper'); +const mailer = require('../services/mailer'); +const kue = require('../services/kue'); const mongoose = require('../services/mongoose'); const util = require('../util'); @@ -119,15 +121,18 @@ startApp(); // Enable job processing on the thread if enabled. if (program.jobs) { - // Start the processor. + // Start the scraper processor. scraper.process(); + + // Start the mail processor. + mailer.process(); } // Define a safe shutdown function to call in the event we need to shutdown // because the node hooks are below which will interrupt the shutdown process. // Shutdown the mongoose connection, the app server, and the scraper. util.onshutdown([ - () => program.jobs ? scraper.shutdown() : null, + () => program.jobs ? kue.Task.shutdown() : null, () => mongoose.disconnect(), () => server.close() ]); diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index c5390607c..eebc0d3ce 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -92,7 +92,7 @@ const forgotPassowordFailure = () => ({type: actions.FETCH_FORGOT_PASSWORD_FAILU export const fetchForgotPassword = email => dispatch => { dispatch(forgotPassowordRequest(email)); - coralApi('/users/request-password-reset', {method: 'POST', body: {email}}) + coralApi('/account/password/reset', {method: 'POST', body: {email}}) .then(() => dispatch(forgotPassowordSuccess())) .catch(error => dispatch(forgotPassowordFailure(error))); }; diff --git a/client/coral-framework/actions/user.js b/client/coral-framework/actions/user.js index 0ee8659d8..b4b7b7896 100644 --- a/client/coral-framework/actions/user.js +++ b/client/coral-framework/actions/user.js @@ -14,10 +14,10 @@ const saveBioFailure = error => ({type: actions.SAVE_BIO_FAILURE, error}); export const saveBio = (user_id, formData) => dispatch => { dispatch(saveBioRequest()); - coralApi(`/users/${user_id}/bio`, {method: 'PUT', body: formData}) - .then(({settings}) => { + coralApi('/account/bio', {method: 'PUT', body: formData}) + .then(() => { dispatch(addNotification('success', lang.t('successBioUpdate'))); - dispatch(saveBioSuccess(settings)); + dispatch(saveBioSuccess(formData)); }) .catch(error => dispatch(saveBioFailure(error))); }; diff --git a/client/coral-framework/reducers/user.js b/client/coral-framework/reducers/user.js index bd5f78e87..3970aa679 100644 --- a/client/coral-framework/reducers/user.js +++ b/client/coral-framework/reducers/user.js @@ -31,8 +31,7 @@ export default function user (state = initialState, action) { case authActions.FETCH_SIGNIN_FACEBOOK_FAILURE: return initialState; case actions.SAVE_BIO_SUCCESS: - return state - .set('settings', action.settings); + return state.set('settings', action.settings); case actions.COMMENTS_BY_USER_SUCCESS: return state.set('myComments', action.comments); case assetActions.MULTIPLE_ASSETS_SUCCESS: diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 8fe8627b0..554afb29e 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -583,7 +583,7 @@ paths: description: The user that has been created. schema: $ref: '#/definitions/User' - /users/update-password: + /account/password/reset: post: parameters: - name: body diff --git a/models/action.js b/models/action.js index ee698029d..8e6634072 100644 --- a/models/action.js +++ b/models/action.js @@ -13,7 +13,7 @@ const ActionSchema = new Schema({ item_type: String, item_id: String, user_id: String, - metadata: Object, //Holds arbitrary metadata about the action. + metadata: Schema.Types.Mixed }, { timestamps: { createdAt: 'created_at', diff --git a/models/setting.js b/models/setting.js index e9e38f463..9e71e36aa 100644 --- a/models/setting.js +++ b/models/setting.js @@ -1,7 +1,6 @@ const mongoose = require('../services/mongoose'); const Schema = mongoose.Schema; const _ = require('lodash'); -const cache = require('../services/cache'); const WordlistSchema = new Schema({ banned: [String], @@ -53,6 +52,10 @@ const SettingSchema = new Schema({ charCountEnable: { type: Boolean, default: false + }, + requireEmailConfirmation: { + type: Boolean, + default: false } }, { timestamps: { @@ -121,19 +124,11 @@ const SettingService = module.exports = {}; */ const selector = {id: '1'}; -/** - * Cache expiry time in seconds for when the cached entry of the settings object - * expires. 2 minutes. - */ -const EXPIRY_TIME = 60 * 2; - /** * Gets the entire settings record and sends it back * @return {Promise} settings the whole settings record */ -SettingService.retrieve = () => cache.wrap('settings', EXPIRY_TIME, () => { - return Setting.findOne(selector); -}).then((setting) => new Setting(setting)); +SettingService.retrieve = () => Setting.findOne(selector); /** * This will update the settings object with whatever you pass in @@ -146,12 +141,6 @@ SettingService.update = (settings) => Setting.findOneAndUpdate(selector, { upsert: true, new: true, setDefaultsOnInsert: true -}).then((settings) => { - - // Invalidate the settings cache. - return cache - .set('settings', settings, EXPIRY_TIME) - .then(() => settings); }); /** diff --git a/models/user.js b/models/user.js index 7df713337..0f8fa3e79 100644 --- a/models/user.js +++ b/models/user.js @@ -4,7 +4,6 @@ const _ = require('lodash'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const Action = require('./action'); - const Comment = require('./comment'); // SALT_ROUNDS is the number of rounds that the bcrypt algorithm will run @@ -31,6 +30,37 @@ if (process.env.NODE_ENV === 'test' && !process.env.TALK_SESSION_SECRET) { throw new Error('TALK_SESSION_SECRET must be defined to encode JSON Web Tokens and other auth functionality'); } +// ProfileSchema is the mongoose schema defined as the representation of a +// User's profile stored in MongoDB. +const ProfileSchema = new mongoose.Schema({ + + // ID provides the identifier for the user profile, in the case of a local + // provider, the id would be an email, in the case of a social provider, + // the id would be the foreign providers identifier. + id: { + type: String, + required: true + }, + + // Provider is simply the name attached to the authentication mode. In the + // case of a locally provided profile, this will simply be `local`, or a + // social provider which for Facebook would just be `facebook`. + provider: { + type: String, + required: true + }, + + // Metadata provides a place to put provider specific details. An example of + // something that could be stored here is the `metadata.confirmed_at` could be + // used by the `local` provider to indicate when the email address was + // confirmed. + metadata: { + type: mongoose.Schema.Types.Mixed + } +}, { + _id: false +}); + // UserSchema is the mongoose schema defined as the representation of a User in // MongoDB. const UserSchema = new mongoose.Schema({ @@ -60,26 +90,7 @@ const UserSchema = new mongoose.Schema({ // Profiles describes the array of identities for a given user. Any one user // can have multiple profiles associated with them, including multiple email // addresses. - profiles: [new mongoose.Schema({ - - // ID provides the identifier for the user profile, in the case of a local - // provider, the id would be an email, in the case of a social provider, - // the id would be the foreign providers identifier. - id: { - type: String, - required: true - }, - - // Provider is simply the name attached to the authentication mode. In the - // case of a locally provided profile, this will simply be `local`, or a - // social provider which for Facebook would just be `facebook`. - provider: { - type: String, - required: true - } - }, { - _id: false - })], + profiles: [ProfileSchema], // Roles provides an array of roles (as strings) that is associated with a // user. @@ -499,45 +510,43 @@ UserService.createPasswordResetToken = function (email) { email = email.toLowerCase(); return UserModel.findOne({profiles: {$elemMatch: {id: email}}}) - .then(user => { + .then((user) => { + if (!user) { - if (user === null) { - - // since we don't want to reveal that the email does/doesn't exist - // just go ahead and resolve the Promise with null and check in the endpoint - return Promise.resolve(null); + // Since we don't want to reveal that the email does/doesn't exist + // just go ahead and resolve the Promise with null and check in the + // endpoint. + return; } - const payload = {email, jti: uuid.v4(), userId: user.id, version: user.__v}; - const token = jwt.sign(payload, process.env.TALK_SESSION_SECRET, {expiresIn: '1d'}); + const payload = { + jti: uuid.v4(), + email, + userId: user.id, + version: user.__v + }; - return token; + return jwt.sign(payload, process.env.TALK_SESSION_SECRET, {algorithm: 'HS256'}, { + expiresIn: '1d' + }); }); }; /** - * verifies a jwt and returns the associated user + * Verifies a jwt and returns the associated user. * @param {String} token the JSON Web Token to verify */ UserService.verifyPasswordResetToken = token => { return new Promise((resolve, reject) => { - jwt.verify(token, process.env.TALK_SESSION_SECRET, (error, decoded) => { - if (error) { - return reject(error); + jwt.verify(token, process.env.TALK_SESSION_SECRET, (err, decoded) => { + if (err) { + return reject(err); } resolve(decoded); }); }) - .then(decoded => { - - /** - * TODO: check the jti from this decoded token in redis - * and make an entry if it does not exist. - * reject if entry already exists. - */ - return UserService.findById(decoded.userId); - }); + .then(decoded => UserService.findById(decoded.userId)); }; /** @@ -594,18 +603,13 @@ UserService.all = () => { * Adds a new User bio * @return {Promise} */ - -UserService.addBio = (id, bio) => ( - UserModel.findOneAndUpdate({ - id - }, { - $set: { - 'settings.bio': bio - } - }, { - new: true - }) -); +UserService.addBio = (id, bio) => UserModel.update({ + id +}, { + $set: { + 'settings.bio': bio + } +}); /** * Add an action to the user. diff --git a/routes/api/account/index.js b/routes/api/account/index.js new file mode 100644 index 000000000..b975ba284 --- /dev/null +++ b/routes/api/account/index.js @@ -0,0 +1,118 @@ +const express = require('express'); +const router = express.Router(); +const User = require('../../../models/user'); +const mailer = require('../../../services/mailer'); +const authorization = require('../../../middleware/authorization'); + +router.get('/', authorization.needed(), (req, res, next) => { + res.json(req.user); +}); + +/** + * this endpoint takes an email (username) and checks if it belongs to a User account + * if it does, create a JWT and send an email + */ +router.post('/password/reset', (req, res, next) => { + const {email} = req.body; + + if (!email) { + return next('you must submit an email when requesting a password.'); + } + + User + .createPasswordResetToken(email) + .then((token) => { + + // Check to see if the token isn't defined. + if (!token) { + + // As it isn't, don't send any emails! + return; + } + + return mailer.sendSimple({ + app: req.app, // needed to render the templates. + template: 'email/password-reset', // needed to know which template to render! + locals: { // specifies the template locals. + token, + rootURL: process.env.TALK_ROOT_URL + }, + subject: 'Password Reset Requested - Talk', + to: email + }); + }) + .then(() => { + + // we want to send a 204 regardless of the user being found in the db + // if we fail on missing emails, it would reveal if people are registered or not. + res.status(204).end(); + }) + .catch((err) => { + next(err); + }); +}); + +// ErrPasswordTooShort is returned when the password length is too short. +const ErrPasswordTooShort = new Error('password must be at least 8 characters'); +ErrPasswordTooShort.status = 400; + +// ErrMissingToken is returned in the event that the password reset is requested +// without a token. +const ErrMissingToken = new Error('token is required'); +ErrMissingToken.status = 400; + +/** + * expects 2 fields in the body of the request + * 1) the token that was in the url of the email link {String} + * 2) the new password {String} + */ +router.put('/password/reset', (req, res, next) => { + + const { + token, + password + } = req.body; + + if (!token) { + return next(ErrMissingToken); + } + + if (!password || password.length < 8) { + return next(ErrPasswordTooShort); + } + + User.verifyPasswordResetToken(token) + .then(user => { + return User.changePassword(user.id, password); + }) + .then(() => { + res.status(204).end(); + }) + .catch(error => { + console.error(error); + + next(authorization.ErrNotAuthorized); + }); +}); + +router.put('/bio', authorization.needed(), (req, res, next) => { + + const { + bio + } = req.body; + + if (!bio) { + return next(new Error('You must submit a new bio')); + } + + User + .addBio(req.user.id, bio) + .then(() => { + res.status(204).end(); + }) + .catch((err) => { + next(err); + }); +}); + +module.exports = router; diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index bcbfcc27e..c798b0ebc 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -31,6 +31,10 @@ router.delete('/', authorization.needed(), (req, res) => { }); }); +//============================================================================== +// PASSPORT ROUTES +//============================================================================== + /** * This sends back the user data as JSON. */ diff --git a/routes/api/index.js b/routes/api/index.js index 316d284e0..120fd47ec 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -17,6 +17,7 @@ router.use('/actions', authorization.needed(), require('./actions')); router.use('/auth', require('./auth')); router.use('/stream', require('./stream')); router.use('/users', require('./users')); +router.use('/account', require('./account')); // Bind the kue handler to the /kue path. router.use('/kue', authorization.needed('admin'), require('../../services/kue').kue.app); diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 1ce2e0e26..8dc48d5dc 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -1,12 +1,6 @@ const express = require('express'); const router = express.Router(); const User = require('../../../models/user'); -const mailer = require('../../../services/mailer'); -const ejs = require('ejs'); -const fs = require('fs'); -const path = require('path'); -const resetEmailFile = fs.readFileSync(path.resolve(__dirname, '../../../views/password-reset-email.ejs')); -const resetEmailTemplate = ejs.compile(resetEmailFile.toString()); const authorization = require('../../../middleware/authorization'); router.get('/', authorization.needed('admin'), (req, res, next) => { @@ -50,19 +44,22 @@ router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => router.post('/:user_id/status', (req, res, next) => { User .setStatus(req.params.user_id, req.body.status, req.body.comment_id) - .then(status => { - res.json(status); + .then((status) => { + res.status(201).json(status); }) .catch(next); }); -router.post('/', (req, res, next) => { - const {email, password, displayName} = req.body; +router.post('/', authorization.needed('admin'), (req, res, next) => { + const { + email, + password, + displayName + } = req.body; User .createLocalUser(email, password, displayName) .then(user => { - res.status(201).json(user); }) .catch(err => { @@ -70,96 +67,7 @@ router.post('/', (req, res, next) => { }); }); -const ErrPasswordTooShort = new Error('password must be at least 8 characters'); -ErrPasswordTooShort.status = 400; - -/** - * expects 2 fields in the body of the request - * 1) the token that was in the url of the email link {String} - * 2) the new password {String} - */ -router.post('/update-password', (req, res, next) => { - const {token, password} = req.body; - - if (!password || password.length < 8) { - return next(ErrPasswordTooShort); - } - - User.verifyPasswordResetToken(token) - .then(user => { - return User.changePassword(user.id, password); - }) - .then(() => { - res.status(204).end(); - }) - .catch(error => { - console.error(error); - - next(authorization.ErrNotAuthorized); - }); -}); - -/** - * this endpoint takes an email (username) and checks if it belongs to a User account - * if it does, create a JWT and send an email - */ -router.post('/request-password-reset', (req, res, next) => { - const {email} = req.body; - - if (!email) { - return next('you must submit an email when requesting a password.'); - } - - User - .createPasswordResetToken(email) - .then(token => { - if (token === null) { - return Promise.resolve('the email was not found in the db.'); - } - - const options = { - subject: 'Password Reset Requested - Talk', - from: process.env.TALK_SMTP_FROM_ADDRESS, - to: email, - html: resetEmailTemplate({ - token, - - // probably more clear to explicitly pass this - rootURL: process.env.TALK_ROOT_URL - }) - }; - return mailer.sendSimple(options); - }) - .then(() => { - - // we want to send a 204 regardless of the user being found in the db - // if we fail on missing emails, it would reveal if people are registered or not. - res.status(204).end(); - }) - .catch((err) => { - next(err); - }); -}); - -router.put('/:user_id/bio', (req, res, next) => { - const {user_id} = req.params; - const {bio} = req.body; - - if (!bio) { - return next('You must submit a new bio'); - } - - User - .addBio(user_id, bio) - .then(user => { - res.json(user); - }) - .catch((err) => { - next(err); - }); -}); - -router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { +router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { const { action_type, metadata diff --git a/scripts/pree2e.sh b/scripts/pree2e.sh index e79addc2e..d8624e81d 100755 --- a/scripts/pree2e.sh +++ b/scripts/pree2e.sh @@ -4,12 +4,12 @@ selenium-standalone install # Creating Admin Test User -{ echo admin@test.com; echo test; echo test; echo Admin Test User; echo admin;} | dotenv ./bin/cli-users create +{ echo admin@test.com; echo test; echo test; echo Admin Test User; echo admin;} | ./bin/cli-users create # Creating Moderator Test User -{ echo moderator@test.com; echo test; echo test; echo Moderator Test User; echo moderator;} | dotenv ./bin/cli-users create +{ echo moderator@test.com; echo test; echo test; echo Moderator Test User; echo moderator;} | ./bin/cli-users create # Creating Commenter Test User -{ echo commenter@test.com; echo test; echo test; echo Commenter Test User; echo ;} | dotenv ./bin/cli-users create +{ echo commenter@test.com; echo test; echo test; echo Commenter Test User; echo ;} | ./bin/cli-users create -npm start +npm start & diff --git a/services/kue.js b/services/kue.js index e2229d424..e9fbe43e8 100644 --- a/services/kue.js +++ b/services/kue.js @@ -1,11 +1,79 @@ -const kue = require('kue'); +const debug = require('debug')('talk:services:kue'); const redis = require('./redis'); -module.exports = { - queue: kue.createQueue({ - redis: { - createClientFactory: () => redis.createClient() - } - }), - kue +module.exports = {}; + +const kue = module.exports.kue = require('kue'); + +// Note that unlike what the name createQueue suggests, it currently returns a +// singleton Queue instance. So you can configure and use only a single Queue +// object within your node.js process. +const Queue = module.exports.queue = kue.createQueue({ + redis: { + createClientFactory: () => redis.createClient() + } +}); + +module.exports.Task = class Task { + + constructor({name, attempts = 3, delay = 1000}) { + this.name = name; + this.attempts = attempts; + this.delay = delay; + } + + /** + * Add a new job to the queue. + */ + create(data) { + + debug(`Creating new job for Queue[${this.name}]`); + + return new Promise((resolve, reject) => { + let job = Queue + .create(this.name, data) + .attempts(this.attempts) + .delay(this.delay) + .backoff({type: 'exponential'}) + .save((err) => { + if (err) { + return reject(err); + } + + debug(`Job[${job.id}] created on Queue[${this.name}]`); + + return resolve(job); + }); + }); + } + + /** + * Process jobs for the queue. + */ + process(callback) { + return Queue.process(this.name, callback); + } + + /** + * Shutdown running jobs. + */ + static shutdown() { + + debug('Shutting down the Queue'); + + return new Promise((resolve, reject) => { + + // Shutdown and give the queue 5 seconds to shutdown before we start + // killing jobs. + Queue.shutdown(5000, (err) => { + if (err) { + return reject(err); + } + + debug('Queue shut down.'); + + resolve(); + }); + }); + } }; diff --git a/services/mailer.js b/services/mailer.js index 9ff7291d4..6fef876d6 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,4 +1,6 @@ +const debug = require('debug')('talk:services:mailer'); const nodemailer = require('nodemailer'); +const kue = require('./kue'); const smtpRequiredProps = [ 'TALK_SMTP_FROM_ADDRESS', @@ -7,11 +9,9 @@ const smtpRequiredProps = [ 'TALK_SMTP_HOST' ]; -smtpRequiredProps.forEach(prop => { - if (!process.env[prop]) { - console.error(`process.env.${prop} should be defined if you would like to send password reset emails from Talk`); - } -}); +if (smtpRequiredProps.some(prop => !process.env[prop])) { + console.error(`${smtpRequiredProps.join(', ')} should be defined in the environment if you would like to send password reset emails from Talk`); +} const options = { host: process.env.TALK_SMTP_HOST, @@ -29,29 +29,89 @@ if (process.env.TALK_SMTP_PORT) { const defaultTransporter = nodemailer.createTransport(options); -const mailer = { +const mailer = module.exports = { /** - * sendSimple - * - * @param {Object} {from, to, subject, text = '', html = ''} - * @returns - */ - sendSimple({from, to, subject, text = '', html = '', transporter = defaultTransporter}) { - return new Promise((resolve, reject) => { - if (!from) { - reject('sendSimple requires a from address'); - } - if (!to) { - reject('sendSimple requires a comma-separated list of "to" addresses'); - } - if (!subject) { - reject('sendSimple requires a subject for the email'); - } + * Create the new Task kue. + */ + task: new kue.Task({ + name: 'mailer' + }), - return resolve(transporter.sendMail({from, to, subject, text, html})); + /** + * Render renders the template with the given locals and returns the rendered + * html/text. + */ + render(app, template, locals = {}) { + return new Promise((resolve, reject) => { + + // Render the template with the app.render method. + app.render(template, locals, (err, rendered) => { + if (err) { + return reject(err); + } + + return resolve(rendered); + }); + }); + }, + + sendSimple({app, template, locals, to, subject}) { + if (!to) { + return Promise.reject('sendSimple requires a comma-separated list of "to" addresses'); + } + + if (!subject) { + return Promise.reject('sendSimple requires a subject for the email'); + } + + return Promise.all([ + + // Render the HTML version of the email. + mailer.render(app, template, locals), + + // Render the TEXT version of the email. + mailer.render(app, `${template}.txt`, locals) + ]) + .then(([html, text]) => { + + // Create the job. + return mailer.task.create({ + title: 'Mail', + message: { + to, + subject, + text, + html + } + }); + }); + }, + + /** + * Start the queue processor for the mailer job. + */ + process() { + + debug(`Now processing ${mailer.task.name} jobs`); + + return mailer.task.process(({id, data}, done) => { + debug(`Starting to send mail for Job[${id}]`); + + // Set the `from` field. + data.message.from = process.env.TALK_SMTP_FROM_ADDRESS; + + // Actually send the email. + defaultTransporter.sendMail(data.message, (err) => { + if (err) { + debug(`Failed to send mail for Job[${id}]:`, err); + return done(err); + } + + debug(`Finished sending mail for Job[${id}]`); + return done(); + }); }); } -}; -module.exports = mailer; +}; diff --git a/services/mongoose.js b/services/mongoose.js index 03c87b937..1b9097760 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -1,21 +1,53 @@ const mongoose = require('mongoose'); const debug = require('debug')('talk:db'); +const queryDebuger = require('debug')('talk:db:query'); + +// Loading the formatter from Mongoose: +// +// https://github.com/Automattic/mongoose/blob/1a93d1f4d12e441e17ddf451e96fbc5f6e8f54b8/lib/drivers/node-mongodb-native/collection.js#L182 +// +// so we can wrap parameters. +const formatter = require('mongoose').Collection.prototype.$format; + +// Provide a newly wrapped debugQuery function which wraps the `debug` package. +function debugQuery(name, i, ...args) { + let functionCall = ['db', name, i].join('.'); + let _args = []; + for (let j = args.length - 1; j >= 0; --j) { + if (formatter(args[j]) || _args.length) { + _args.unshift(formatter(args[j])); + } + } + + let params = `(${_args.join(', ')})`; + + queryDebuger(functionCall + params); +} + const enabled = require('debug').enabled; -// Append '-test' to the db if node_env === 'test' -let url = process.env.TALK_MONGO_URL || 'mongodb://localhost/coral-talk'; +// Pull the mongo url out of the environment. +let url = process.env.TALK_MONGO_URL; -if (process.env.NODE_ENV === 'test') { - url += '-test'; +// Reset the mongo url in the event it hasn't been overrided and we are in a +// testing environment. Every new mongo instance comes with a test database by +// default, this is consistent with common testing and use case practices. +if (process.env.NODE_ENV === 'test' && !url) { + url = 'mongodb://localhost/test'; } // Use native promises mongoose.Promise = global.Promise; +// Check if debugging is enabled on the talk:db prefix. if (enabled('talk:db')) { - mongoose.set('debug', true); + + // Enable the mongoose debugger, here we wrap the similar print function + // provided by setting the debug parameter. + mongoose.set('debug', debugQuery); } +// Connect to the Mongo instance. mongoose.connect(url, (err) => { if (err) { throw err; diff --git a/services/passport.js b/services/passport.js index f709970dc..130de234d 100644 --- a/services/passport.js +++ b/services/passport.js @@ -1,5 +1,6 @@ const passport = require('passport'); const User = require('../models/user'); +const Setting = require('../models/setting'); const LocalStrategy = require('passport-local').Strategy; const FacebookStrategy = require('passport-facebook').Strategy; @@ -27,7 +28,7 @@ passport.deserializeUser((id, done) => { * @param {User} user the user to be validated * @param {Function} done the callback for the validation */ -function ValidateUserLogin(user, done) { +function ValidateUserLogin(loginProfile, user, done) { if (!user) { return done(new Error('user not found')); } @@ -36,7 +37,36 @@ function ValidateUserLogin(user, done) { return done(null, false, {message: 'Account disabled'}); } - return done(null, user); + // If the user isn't a local user (i.e., a social user). + if (loginProfile.provider !== 'local') { + return done(null, user); + } + + // The user is a local user, check if we need email confirmation. + return Setting.retrieve().then(({requireEmailConfirmation = false}) => { + + // If we have the requirement of checking that emails for users are + // verified, then we need to check the email address to ensure that it has + // been verified. + if (requireEmailConfirmation) { + + // Get the profile representing the local account. + let profile = user.profiles.find((profile) => profile.id === loginProfile.id); + + // This should never get to this point, if it does, don't let this past. + if (!profile) { + throw new Error('ID indicated by loginProfile is not on user object'); + } + + // If the profile doesn't have a metadata field, or it does not have a + // confirmed_at field, or that field is null, then send them back. + if (!profile.metadata || !profile.metadata.confirmed_at || profile.metadata.confirmed_at === null) { + return done(null, false, {message: `Email address ${loginProfile.id} not verified.`}); + } + } + + return done(null, user); + }); } //============================================================================== @@ -54,7 +84,12 @@ passport.use(new LocalStrategy({ return done(null, false, {message: 'Incorrect email/password combination'}); } - return ValidateUserLogin(user, done); + // Define the loginProfile being used to perform an additional + // verificaiton. + let loginProfile = {id: email, provider: 'local'}; + + // Validate the user login. + return ValidateUserLogin(loginProfile, user, done); }) .catch((err) => { done(err); @@ -70,9 +105,9 @@ if (process.env.TALK_FACEBOOK_APP_ID && process.env.TALK_FACEBOOK_APP_SECRET && }, (accessToken, refreshToken, profile, done) => { User .findOrCreateExternalUser(profile) - .then((user) => - ValidateUserLogin(user, done) - ) + .then((user) => { + return ValidateUserLogin(profile, user, done); + }) .catch((err) => { done(err); }); diff --git a/services/scraper.js b/services/scraper.js index c75a12750..6a3260757 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -1,7 +1,6 @@ const kue = require('./kue'); const debug = require('debug')('talk:services:scraper'); const Asset = require('../models/asset'); -const JOB_NAME = 'scraper'; const metascraper = require('metascraper'); @@ -12,29 +11,27 @@ const metascraper = require('metascraper'); const scraper = { /** - * creates a new scraper job and scrapes the url when it gets processed. + * Create the new Task kue. + */ + task: new kue.Task({ + name: 'scraper' + }), + + /** + * Creates a new scraper job and scrapes the url when it gets processed. */ create(asset) { - return new Promise((resolve, reject) => { - debug(`Creating job for Asset[${asset.id}]`); - let job = kue.queue - .create(JOB_NAME, { - title: `Scrape for asset ${asset.id}`, - asset_id: asset.id - }) - .attempts(3) - .delay(1000) - .backoff({type: 'exponential'}) - .save((err) => { - if (err) { - return reject(err); - } + debug(`Creating job for Asset[${asset.id}]`); - debug(`Created Job[${job.id}] for Asset[${asset.id}]`); + return scraper.task.create({ + title: `Scrape for asset ${asset.id}`, + asset_id: asset.id + }).then((job) => { - return resolve(job); - }); + debug(`Created Job[${job.id}] for Asset[${asset.id}]`); + + return job; }); }, @@ -48,6 +45,9 @@ const scraper = { })); }, + /** + * Updates an Asset based on scraped asset metadata. + */ update(id, meta) { return Asset.update({id}, { $set: { @@ -68,10 +68,9 @@ const scraper = { */ process() { - debug(`Now processing ${JOB_NAME} jobs`); + debug(`Now processing ${scraper.task.name} jobs`); - // Process jobs with the processJob function. - kue.queue.process(JOB_NAME, (job, done) => { + scraper.task.process((job, done) => { debug(`Starting on Job[${job.id}] for Asset[${job.data.asset_id}]`); @@ -111,27 +110,6 @@ const scraper = { done(err); }); }); - }, - - /** - * Shuts down the current queue to ensure that the application can shutdown - * cleanly. - */ - shutdown() { - return new Promise((resolve, reject) => { - - // Shutdown and give the queue 5 seconds to shutdown before we start - // killing jobs. - kue.queue.shutdown(5000, (err) => { - if (err) { - return reject(err); - } - - debug(`Processing for ${JOB_NAME} jobs stopped`); - - resolve(); - }); - }); } }; diff --git a/tests/routes/api/auth/index.js b/tests/routes/api/auth/index.js index dd408d135..ca7b0d558 100644 --- a/tests/routes/api/auth/index.js +++ b/tests/routes/api/auth/index.js @@ -19,22 +19,29 @@ describe('/api/v1/auth', () => { }); }); +const Setting = require('../../../../models/setting'); +const settings = {id: '1'}; + describe('/api/v1/auth/local', () => { - beforeEach(() => { - return User.createLocalUser('maria@gmail.com', 'password!', 'Maria'); - }); + beforeEach(() => Promise.all([ + User.createLocalUser('maria@gmail.com', 'password!', 'Maria'), + Setting.init(settings) + ])); describe('#post', () => { it('should send back the user on a successful login', () => { return chai.request(app) .post('/api/v1/auth/local') .send({email: 'maria@gmail.com', password: 'password!'}) - .catch((res) => { + .then((res) => { expect(res).to.have.status(200); expect(res).to.be.json; expect(res.body).to.have.property('user'); expect(res.body.user).to.have.property('displayName', 'Maria'); + }) + .catch((err) => { + console.error(err); }); }); diff --git a/views/password-reset-email.ejs b/views/email/password-reset.ejs similarity index 58% rename from views/password-reset-email.ejs rename to views/email/password-reset.ejs index 17ed9e39b..e478ceeba 100644 --- a/views/password-reset-email.ejs +++ b/views/email/password-reset.ejs @@ -1,6 +1,2 @@ -

We received a request to reset your password. If you did not request this change, you can ignore this email.
If you did, please click here to reset password.

-<% if (process.env.NODE_ENV !== 'production') { %> -

<%= token %>

-<% } %> diff --git a/views/email/password-reset.txt.ejs b/views/email/password-reset.txt.ejs new file mode 100644 index 000000000..1e44a6629 --- /dev/null +++ b/views/email/password-reset.txt.ejs @@ -0,0 +1,5 @@ +We received a request to reset your password, click here to reset your password: + +<%= rootURL %>/admin/password-reset#<%= token %> + +If you did not request this change, you can ignore this email. diff --git a/views/password-reset.ejs b/views/password-reset.ejs index 1ffd1b554..8ad289dff 100644 --- a/views/password-reset.ejs +++ b/views/password-reset.ejs @@ -117,9 +117,9 @@ } $.ajax({ - url: '/api/v1/users/update-password', + url: '/api/v1/account/password/reset', contentType: 'application/json', - method: 'POST', + method: 'PUT', data: JSON.stringify({password: password, token: location.hash.replace('#', '')}) }).then(function (success) { location.href = '<%= redirectUri %>'; From 7b9dbb5afd3618104dc1b2b06bfb7617471e3da1 Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 12:24:01 -0300 Subject: [PATCH 36/58] Review changes. --- app.js | 6 ++++-- client/coral-admin/src/actions/users.js | 2 +- client/coral-framework/actions/auth.js | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app.js b/app.js index 602c36320..eb515a0f1 100644 --- a/app.js +++ b/app.js @@ -73,8 +73,10 @@ app.use(session(session_opts)); app.use(cookieParser()); app.use((err, req, res, next) => { - res.locals._csrf = req.csrfToken(); - return next(); + if (req.method === 'POST') { + res.locals._csrf = req.csrfToken(); + } + next(); }); //============================================================================== diff --git a/client/coral-admin/src/actions/users.js b/client/coral-admin/src/actions/users.js index e570be292..30c20290f 100644 --- a/client/coral-admin/src/actions/users.js +++ b/client/coral-admin/src/actions/users.js @@ -9,7 +9,7 @@ export const userStatusUpdate = (status, userId, commentId) => { return (dispatch, getState) => { dispatch({type: actions.UPDATE_STATUS_REQUEST}); const _csrf = getState().auth.get('_csrf'); - return coralApi(`/users/${userId}/status`, {method: 'POST', body: {status: status, comment_id: commentId}, _csrf: _csrf}) + return coralApi(`/users/${userId}/status`, {method: 'POST', body: {status: status, comment_id: commentId}, _csrf}) .then(res => dispatch({type: actions.UPDATE_STATUS_SUCCESS, res})) .catch(error => dispatch({type: actions.UPDATE_STATUS_FAILURE, error})); }; diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index 28e4a6a3c..581fb729c 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -26,7 +26,7 @@ const signInFailure = error => ({type: actions.FETCH_SIGNIN_FAILURE, error}); export const fetchSignIn = (formData) => (dispatch, getState) => { dispatch(signInRequest()); const _csrf = getState().auth.get('_csrf'); - coralApi('/auth/local', {method: 'POST', body: formData, _csrf: _csrf}) + coralApi('/auth/local', {method: 'POST', body: formData, _csrf}) .then(({user}) => { const isAdmin = !!user.roles.filter(i => i === 'admin').length; dispatch(signInSuccess(user, isAdmin)); @@ -77,7 +77,7 @@ export const fetchSignUp = formData => (dispatch, getState) => { dispatch(signUpRequest()); const _csrf = getState().auth.get('_csrf'); - coralApi('/users', {method: 'POST', body: formData, _csrf: _csrf}) + coralApi('/users', {method: 'POST', body: formData, _csrf}) .then(({user}) => { dispatch(signUpSuccess(user)); setTimeout(() =>{ @@ -97,7 +97,7 @@ export const fetchForgotPassword = email => (dispatch, getState) => { dispatch(forgotPassowordRequest(email)); const _csrf = getState().auth.get('_csrf'); - coralApi('/users/request-password-reset', {method: 'POST', body: {email}, _csrf: _csrf}) + coralApi('/users/request-password-reset', {method: 'POST', body: {email}, _csrf}) .then(() => dispatch(forgotPassowordSuccess())) .catch(error => dispatch(forgotPassowordFailure(error))); }; From ccb9fe316592acc4ddb8c90808c1d7528fe36568 Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 12:40:25 -0300 Subject: [PATCH 37/58] Adds DELETE and PUT. --- app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.js b/app.js index eb515a0f1..7a05f81df 100644 --- a/app.js +++ b/app.js @@ -73,7 +73,7 @@ app.use(session(session_opts)); app.use(cookieParser()); app.use((err, req, res, next) => { - if (req.method === 'POST') { + if (req.method === 'POST' || req.method === 'PUT' || req.method === 'DELETE') { res.locals._csrf = req.csrfToken(); } next(); From 3718efce1e7862cc9ea23c88b6a94482e5238fdf Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 12:47:19 -0300 Subject: [PATCH 38/58] Adds cookie parser --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 44cd789b4..9cedc7a82 100644 --- a/package.json +++ b/package.json @@ -92,6 +92,7 @@ "babel-preset-stage-0": "^6.16.0", "chai": "^3.5.0", "chai-http": "^3.0.0", + "cookie-parser": "^1.4.3", "copy-webpack-plugin": "^4.0.0", "csurf": "^1.9.0", "css-loader": "^0.25.0", From 5f7d48a7f9dae3f91dfeef7962ea25a4bac5d851 Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 13:17:16 -0300 Subject: [PATCH 39/58] This was not needed. --- app.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app.js b/app.js index 7a05f81df..d230596c0 100644 --- a/app.js +++ b/app.js @@ -72,13 +72,6 @@ app.use(session(session_opts)); app.use(cookieParser()); -app.use((err, req, res, next) => { - if (req.method === 'POST' || req.method === 'PUT' || req.method === 'DELETE') { - res.locals._csrf = req.csrfToken(); - } - next(); -}); - //============================================================================== // PASSPORT MIDDLEWARE //============================================================================== From 298e1e8d7366759321329a7acf58f3a0d89112f2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 4 Jan 2017 10:16:51 -0700 Subject: [PATCH 40/58] Updates to user cli + e2e + tests - Updates to before + beforeEach for mongooose - Removed reference to dotenv from cli in e2e, should use NODE_ENV=test instead. - Changed test port from 30?? to 3000 to be consistent with what nightwatch was expecting --- bin/cli-serve | 2 +- bin/cli-users | 16 +++++--- scripts/pree2e.sh | 6 +-- tests/e2e/tests/Visitor/SignUpTest.js | 5 ++- tests/models/action.js | 42 +++++++++++--------- tests/mongoose.js | 55 +++++++++++++++++++-------- 6 files changed, 80 insertions(+), 46 deletions(-) diff --git a/bin/cli-serve b/bin/cli-serve index 5095c6362..c8cf37a47 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -14,7 +14,7 @@ const util = require('../util'); * Get port from environment and store in Express. */ -const port = normalizePort(process.env.TALK_PORT || (process.env.NODE_ENV === 'test' ? '3011' : '3000')); +const port = normalizePort(process.env.TALK_PORT || '3000'); app.set('port', port); diff --git a/bin/cli-users b/bin/cli-users index ae688e122..fd9e30c7f 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -80,12 +80,16 @@ function createUser(options) { .then((user) => { console.log(`Created user ${user.id}.`); - return User - .addRoleToUser(user.id, result.role.trim()) - .then(() => { - console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); - util.shutdown(); - }); + if (result.role && result.role.length > 0) { + return User + .addRoleToUser(user.id, result.role.trim()) + .then(() => { + console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); + util.shutdown(); + }); + } else { + util.shutdown(); + } }) .catch((err) => { console.error(err); diff --git a/scripts/pree2e.sh b/scripts/pree2e.sh index d8624e81d..0487a9187 100755 --- a/scripts/pree2e.sh +++ b/scripts/pree2e.sh @@ -4,12 +4,12 @@ selenium-standalone install # Creating Admin Test User -{ echo admin@test.com; echo test; echo test; echo Admin Test User; echo admin;} | ./bin/cli-users create +./bin/cli-users create --flag_mode --email "admin@test.com" --password "test" --name "Admin Test User" --role "admin" # Creating Moderator Test User -{ echo moderator@test.com; echo test; echo test; echo Moderator Test User; echo moderator;} | ./bin/cli-users create +./bin/cli-users create --flag_mode --email "moderator@test.com" --password "test" --name "Moderator Test User" --role "moderator" # Creating Commenter Test User -{ echo commenter@test.com; echo test; echo test; echo Commenter Test User; echo ;} | ./bin/cli-users create +./bin/cli-users create --flag_mode --email "commenter@test.com" --password "test" --name "commenter@test.com" npm start & diff --git a/tests/e2e/tests/Visitor/SignUpTest.js b/tests/e2e/tests/Visitor/SignUpTest.js index 4c7650630..9399b47ca 100644 --- a/tests/e2e/tests/Visitor/SignUpTest.js +++ b/tests/e2e/tests/Visitor/SignUpTest.js @@ -1,3 +1,5 @@ +const uuid = require('uuid'); + module.exports = { '@tags': ['signup', 'visitor'], before: client => { @@ -9,11 +11,10 @@ module.exports = { }, 'Visitor signs up': client => { const embedStreamPage = client.page.embedStreamPage(); - const hash = Math.floor(Math.random() * (999 - 0)); embedStreamPage .signUp({ - email: `visitor_${hash}@test.com`, + email: `visitor_${uuid.v4()}@test.com`, displayName: 'Visitor', pass: 'testtest' }); diff --git a/tests/models/action.js b/tests/models/action.js index 5f05dea71..3af55a640 100644 --- a/tests/models/action.js +++ b/tests/models/action.js @@ -4,24 +4,30 @@ const expect = require('chai').expect; describe('models.Action', () => { let mockActions = []; - beforeEach(() => Action.create([{ - action_type: 'flag', - item_id: '123', - item_type: 'comment', - user_id: 'flagginguserid' - }, { - action_type: 'flag', - item_id: '456', - item_type: 'comment' - }, { - action_type: 'flag', - item_id: '123', - item_type: 'comment' - }, { - action_type: 'like', - item_id: '123', - item_type: 'comment' - }]).then((actions) => { + beforeEach(() => Action.create([ + { + action_type: 'flag', + item_id: '123', + item_type: 'comment', + user_id: 'flagginguserid' + }, + { + action_type: 'flag', + item_id: '456', + item_type: 'comment' + }, + { + action_type: 'flag', + item_id: '123', + item_type: 'comment' + }, + { + action_type: 'like', + item_id: '123', + item_type: 'comment' + } + ]).then((actions) => { + console.log('all created'); mockActions = actions; })); diff --git a/tests/mongoose.js b/tests/mongoose.js index 7544072ec..eed246629 100644 --- a/tests/mongoose.js +++ b/tests/mongoose.js @@ -1,27 +1,50 @@ const mongoose = require('../services/mongoose'); -beforeEach(function (done) { - function clearDB() { - for (let collection in mongoose.connection.collections) { - mongoose.connection.collections[collection].remove(function() {}); - } - return done(); - } - - if (mongoose.connection.readyState === 0) { - mongoose.on('open', function() { +function waitTillConnect() { + return new Promise((resolve, reject) => { + mongoose.connection.on('open', function(err) { if (err) { - throw err; + return reject(err); } - return clearDB(); + return resolve(); + }); + }); +} + +before(function(done) { + this.timeout(30000); + + waitTillConnect() + .then(() => { + done(); + }) + .catch((err) => { + done(err); }); - } else { - return clearDB(); - } }); -after(function (done) { +beforeEach(function(done) { + Promise.all(Object.keys(mongoose.connection.collections).map((collection) => { + return new Promise((resolve, reject) => { + mongoose.connection.collections[collection].remove(function(err) { + if (err) { + return reject(err); + } + + return resolve(); + }); + }); + })) + .then(() => { + done(); + }) + .catch((err) => { + done(err); + }); +}); + +after(function(done) { mongoose.disconnect(); return done(); }); From 12f6bb1ffe841d57049ab60ac1988c6d1ca252a7 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 4 Jan 2017 11:14:23 -0700 Subject: [PATCH 41/58] fetch settings on mod queue load --- .../src/containers/ModerationQueue/ModerationContainer.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js index 145faf4df..8a684184a 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js @@ -9,6 +9,7 @@ import { fetchModerationQueueComments } from 'actions/comments'; import {userStatusUpdate} from 'actions/users'; +import {fetchSettings} from 'actions/settings'; import ModerationQueue from './ModerationQueue'; @@ -29,6 +30,7 @@ class ModerationContainer extends React.Component { componentWillMount() { this.props.fetchModerationQueueComments(); + this.props.fetchSettings(); key('s', () => this.setState({singleView: !this.state.singleView})); key('shift+/', () => this.setState({modalOpen: true})); key('esc', () => this.setState({modalOpen: false})); @@ -86,6 +88,7 @@ const mapStateToProps = state => ({ const mapDispatchToProps = dispatch => { return { + fetchSettings: () => dispatch(fetchSettings()), fetchModerationQueueComments: () => dispatch(fetchModerationQueueComments()), showBanUserDialog: (userId, userName, commentId) => dispatch(showBanUserDialog(userId, userName, commentId)), hideBanUserDialog: () => dispatch(hideBanUserDialog(false)), From 864b08135a408e3116fd336d7c250e46e2101001 Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 16:24:17 -0300 Subject: [PATCH 42/58] Moves the packages to dependencies. --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 9cedc7a82..800d945e3 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,8 @@ "cli-table": "^0.3.1", "commander": "^2.9.0", "connect-redis": "^3.1.0", + "cookie-parser": "^1.4.3", + "csurf": "^1.9.0", "debug": "^2.2.0", "ejs": "^2.5.2", "env-rewrite": "^1.0.2", @@ -92,9 +94,7 @@ "babel-preset-stage-0": "^6.16.0", "chai": "^3.5.0", "chai-http": "^3.0.0", - "cookie-parser": "^1.4.3", "copy-webpack-plugin": "^4.0.0", - "csurf": "^1.9.0", "css-loader": "^0.25.0", "dialog-polyfill": "^0.4.4", "enzyme": "^2.6.0", From ab825512b9fee14431d8bc2b15712d0eb7a9ff6d Mon Sep 17 00:00:00 2001 From: gaba Date: Wed, 4 Jan 2017 16:46:19 -0300 Subject: [PATCH 43/58] Adding missing translation. Not sure how to translate streams. --- client/coral-admin/src/translations.json | 1 + 1 file changed, 1 insertion(+) diff --git a/client/coral-admin/src/translations.json b/client/coral-admin/src/translations.json index 3397a521b..4142c109f 100644 --- a/client/coral-admin/src/translations.json +++ b/client/coral-admin/src/translations.json @@ -146,6 +146,7 @@ "moderate": "Moderar", "configure": "Configurar", "community": "Comunidad", + "streams": "Streams", "closed-comments-desc": "Escribe un mensaje para cuando los comentarios se encuentran cerrados", "closed-comments-label": "Escribe un mensaje...", "never": "Nunca", From ac6fba2286a6a7c658d726e6eb04c527776bf009 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 5 Jan 2017 15:34:27 -0300 Subject: [PATCH 44/58] Clean user state on logout. Allow flagging only if not banned. --- client/coral-framework/constants/user.js | 1 + client/coral-framework/reducers/user.js | 2 ++ client/coral-plugin-flags/FlagButton.js | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/client/coral-framework/constants/user.js b/client/coral-framework/constants/user.js index 6e09726d3..9c6f508fe 100644 --- a/client/coral-framework/constants/user.js +++ b/client/coral-framework/constants/user.js @@ -4,3 +4,4 @@ export const SAVE_BIO_FAILURE = 'SAVE_BIO_FAILURE'; export const COMMENTS_BY_USER_REQUEST = 'COMMENTS_BY_USER_REQUEST'; export const COMMENTS_BY_USER_SUCCESS = 'COMMENTS_BY_USER_SUCCESS'; export const COMMENTS_BY_USER_FAILURE = 'COMMENTS_BY_USER_FAILURE'; +export const LOGOUT_SUCCESS = 'LOGOUT_SUCCESS'; diff --git a/client/coral-framework/reducers/user.js b/client/coral-framework/reducers/user.js index bd5f78e87..a6f980fd8 100644 --- a/client/coral-framework/reducers/user.js +++ b/client/coral-framework/reducers/user.js @@ -37,6 +37,8 @@ export default function user (state = initialState, action) { return state.set('myComments', action.comments); case assetActions.MULTIPLE_ASSETS_SUCCESS: return state.set('myAssets', action.assets); + case actions.LOGOUT_SUCCESS: + return initialState; default : return state; } diff --git a/client/coral-plugin-flags/FlagButton.js b/client/coral-plugin-flags/FlagButton.js index 097ac9d5b..23943318d 100644 --- a/client/coral-plugin-flags/FlagButton.js +++ b/client/coral-plugin-flags/FlagButton.js @@ -102,7 +102,7 @@ class FlagButton extends Component { const popupMenu = getPopupMenu[this.state.step](this.state.itemType); return
-