From fb2f8ef07485a1532e99772ac9d4ec6cd31a61f2 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 22 Mar 2017 16:48:28 -0600 Subject: [PATCH 1/3] redirect to the parent url when resetting your password --- client/coral-framework/actions/auth.js | 4 +++- routes/admin/index.js | 2 +- routes/api/account/index.js | 12 ++++++------ services/users.js | 7 +++++-- views/admin/password-reset.ejs | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index 9dedaa2f2..2a0c2a497 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -3,6 +3,7 @@ import translations from './../translations'; const lang = new I18n(translations); import * as actions from '../constants/auth'; import coralApi, {base} from '../helpers/response'; +import {pym} from 'coral-framework'; // Dialog Actions export const showSignInDialog = (offset = 0) => ({type: actions.SHOW_SIGNIN_DIALOG, offset}); @@ -135,7 +136,8 @@ const forgotPassowordFailure = () => ({type: actions.FETCH_FORGOT_PASSWORD_FAILU export const fetchForgotPassword = email => (dispatch) => { dispatch(forgotPassowordRequest(email)); - coralApi('/account/password/reset', {method: 'POST', body: {email}}) + const redirectUri = pym.parentUrl || location.href; + coralApi('/account/password/reset', {method: 'POST', body: {email, loc: redirectUri}}) .then(() => dispatch(forgotPassowordSuccess())) .catch(error => dispatch(forgotPassowordFailure(error))); }; diff --git a/routes/admin/index.js b/routes/admin/index.js index d250ea32a..1a9769591 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -12,7 +12,7 @@ router.get('/password-reset', (req, res) => { // TODO: store the redirect uri in the token or something fancy. // admins and regular users should probably be redirected to different places. - res.render('admin/password-reset', {redirectUri: process.env.TALK_ROOT_URL}); + res.render('admin/password-reset'); }); router.get('*', (req, res) => { diff --git a/routes/api/account/index.js b/routes/api/account/index.js index 44b4b3953..3761aef95 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -41,14 +41,14 @@ router.post('/email/verify', (req, res, next) => { * if it does, create a JWT and send an email */ router.post('/password/reset', (req, res, next) => { - const {email} = req.body; + const {email, loc} = req.body; if (!email) { return next('you must submit an email when requesting a password.'); } UsersService - .createPasswordResetToken(email) + .createPasswordResetToken(email, loc) .then((token) => { // Check to see if the token isn't defined. @@ -101,11 +101,11 @@ router.put('/password/reset', (req, res, next) => { UsersService .verifyPasswordResetToken(token) - .then((user) => { - return UsersService.changePassword(user.id, password); + .then(([user, loc]) => { + return Promise.all([UsersService.changePassword(user.id, password), loc]); }) - .then(() => { - res.status(204).end(); + .then(([_, loc]) => { + res.json({redirect: loc}); }) .catch(() => { next(authorization.ErrNotAuthorized); diff --git a/services/users.js b/services/users.js index 4e1196252..7f83da9f9 100644 --- a/services/users.js +++ b/services/users.js @@ -524,7 +524,7 @@ module.exports = class UsersService { * Creates a JWT from a user email. Only works for local accounts. * @param {String} email of the local user */ - static createPasswordResetToken(email) { + static createPasswordResetToken(email, loc) { if (!email || typeof email !== 'string') { return Promise.reject('email is required when creating a JWT for resetting passord'); } @@ -544,6 +544,7 @@ module.exports = class UsersService { const payload = { jti: uuid.v4(), email, + loc, userId: user.id, version: user.__v }; @@ -588,7 +589,9 @@ module.exports = class UsersService { }) // TODO: add search by __v as well - .then((decoded) => UsersService.findById(decoded.userId)); + .then((decoded) => { + return Promise.all([UsersService.findById(decoded.userId), decoded.loc]); + }); } /** diff --git a/views/admin/password-reset.ejs b/views/admin/password-reset.ejs index 06b9d2a86..4b25d1279 100644 --- a/views/admin/password-reset.ejs +++ b/views/admin/password-reset.ejs @@ -126,7 +126,7 @@ }, data: JSON.stringify({password: password, token: location.hash.replace('#', '')}) }).then(function (success) { - location.href = '<%= redirectUri %>'; + location.href = success.redirect; }).catch(function (error) { showError(error.responseText); }); From 4b0d0fed534ef90c19846bdd573ca8e7171cb374 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Wed, 22 Mar 2017 17:10:11 -0600 Subject: [PATCH 2/3] unused variable --- routes/api/account/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api/account/index.js b/routes/api/account/index.js index 3761aef95..e1c73d638 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -104,7 +104,7 @@ router.put('/password/reset', (req, res, next) => { .then(([user, loc]) => { return Promise.all([UsersService.changePassword(user.id, password), loc]); }) - .then(([_, loc]) => { + .then(([ , loc]) => { res.json({redirect: loc}); }) .catch(() => { From 984514d7ebdf0f76c1cc66fc24b8d98fadb97018 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 23 Mar 2017 12:46:20 -0600 Subject: [PATCH 3/3] check whitelist before redirecting --- services/users.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/services/users.js b/services/users.js index ac1280664..0bce64425 100644 --- a/services/users.js +++ b/services/users.js @@ -1,4 +1,5 @@ const bcrypt = require('bcrypt'); +const url = require('url'); const jwt = require('jsonwebtoken'); const Wordlist = require('./wordlist'); const errors = require('../errors'); @@ -13,6 +14,7 @@ const USER_ROLES = require('../models/user').USER_ROLES; const RECAPTCHA_WINDOW_SECONDS = 60 * 10; // 10 minutes. const RECAPTCHA_INCORRECT_TRIGGER = 5; // after 3 incorrect attempts, recaptcha will be required. +const SettingsService = require('./settings'); const ActionsService = require('./actions'); // In the event that the TALK_SESSION_SECRET is missing but we are testing, then @@ -485,8 +487,11 @@ module.exports = class UsersService { email = email.toLowerCase(); - return UserModel.findOne({profiles: {$elemMatch: {id: email}}}) - .then((user) => { + return Promise.all([ + UserModel.findOne({profiles: {$elemMatch: {id: email}}}), + SettingsService.retrieve() + ]) + .then(([user, settings]) => { if (!user) { // Since we don't want to reveal that the email does/doesn't exist @@ -495,6 +500,17 @@ module.exports = class UsersService { return; } + let redirectDomain; + try { + redirectDomain = url.parse(loc).hostname; + } catch (e) { + return Promise.reject('redirect location is invalid'); + } + + if (settings.domains.whitelist.indexOf(redirectDomain) === -1) { + return Promise.reject('redirect location is not on the list of acceptable domains'); + } + const payload = { jti: uuid.v4(), email,