From 59f700b11025fc2754d44a475c4a85924abeb003 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Fri, 25 May 2018 23:14:09 +0530 Subject: [PATCH 01/10] fix(auth): Add verification route for email --- .../Settings/components/Email-Settings.jsx | 6 +- server/boot/authentication.js | 26 ++++++++- server/boot/user.js | 18 +++--- server/middleware.json | 1 + .../middlewares/email-not-verified-notice.js | 32 +++++++++++ server/utils/middleware.js | 2 +- server/views/account/update-email.jade | 57 +++++++++++++++++++ server/views/partials/navbar.jade | 2 +- 8 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 server/middlewares/email-not-verified-notice.js create mode 100644 server/views/account/update-email.jade diff --git a/common/app/routes/Settings/components/Email-Settings.jsx b/common/app/routes/Settings/components/Email-Settings.jsx index d9f1aac76b..7544f15512 100644 --- a/common/app/routes/Settings/components/Email-Settings.jsx +++ b/common/app/routes/Settings/components/Email-Settings.jsx @@ -132,9 +132,9 @@ class EmailSettings extends PureComponent { - A change of email address has not been verified. - To use your new email, you must verify it first using the link - we sent you. + Your email has not been verified. + To use your email, you must + verify it here first. diff --git a/server/boot/authentication.js b/server/boot/authentication.js index 1a95cb2e40..cf108fa3ba 100644 --- a/server/boot/authentication.js +++ b/server/boot/authentication.js @@ -38,9 +38,33 @@ module.exports = function enableAuthentication(app) { ifUserRedirect, (req, res) => res.redirect(301, '/auth/auth0')); + router.get( + '/update-email', + ifNoUserRedirectHome, + (req, res) => res.render('account/update-email', { + title: 'Update your email' + }) + ); + router.get('/signout', (req, res) => { req.logout(); - res.redirect('/'); + req.session.destroy( (err) => { + if (err) { + throw wrapHandledError( + new Error('could not destroy session'), + { + type: 'info', + message: 'Oops, something is not right.', + redirectTo: '/' + } + ); + } + res.clearCookie('jwt_access_token'); + res.clearCookie('access_token'); + res.clearCookie('userId'); + res.clearCookie('_csrf'); + res.redirect('/'); + }); }); router.get( diff --git a/server/boot/user.js b/server/boot/user.js index 6788e4e7e4..bc05d88470 100644 --- a/server/boot/user.js +++ b/server/boot/user.js @@ -5,12 +5,12 @@ import { curry } from 'lodash'; import { ifNoUser401, ifNoUserRedirectTo, - ifNotVerifiedRedirectToSettings + ifNotVerifiedRedirectToUpdateEmail } from '../utils/middleware'; const debug = debugFactory('fcc:boot:user'); -const sendNonUserToMap = ifNoUserRedirectTo('/map'); -const sendNonUserToMapWithMessage = curry(ifNoUserRedirectTo, 2)('/map'); +const sendNonUserToHome = ifNoUserRedirectTo('/'); +const sendNonUserToHomeWithMessage = curry(ifNoUserRedirectTo, 2)('/'); module.exports = function(app) { const router = app.loopback.Router(); @@ -24,7 +24,7 @@ module.exports = function(app) { ); api.get( '/account', - sendNonUserToMap, + sendNonUserToHome, getAccount ); api.post( @@ -34,15 +34,15 @@ module.exports = function(app) { ); api.get( '/account/unlink/:social', - sendNonUserToMap, + sendNonUserToHome, getUnlinkSocial ); // Ensure these are the last routes! router.get( '/user/:username/report-user/', - sendNonUserToMapWithMessage('You must be signed in to report a user'), - ifNotVerifiedRedirectToSettings, + sendNonUserToHomeWithMessage('You must be signed in to report a user'), + ifNotVerifiedRedirectToUpdateEmail, getReportUserProfile ); @@ -119,6 +119,10 @@ module.exports = function(app) { if (err) { return next(err); } req.logout(); req.flash('success', 'You have successfully deleted your account.'); + res.clearCookie('jwt_access_token'); + res.clearCookie('access_token'); + res.clearCookie('userId'); + res.clearCookie('_csrf'); return res.status(200).end(); }); } diff --git a/server/middleware.json b/server/middleware.json index f6ce16ed6f..c664ae1b0f 100644 --- a/server/middleware.json +++ b/server/middleware.json @@ -58,6 +58,7 @@ "./middlewares/jade-helpers": {}, "./middlewares/flash-cheaters": {}, "./middlewares/passport-login": {}, + "./middlewares/email-not-verified-notice": {}, "./middlewares/privacy-terms-notice": {} }, "files": {}, diff --git a/server/middlewares/email-not-verified-notice.js b/server/middlewares/email-not-verified-notice.js new file mode 100644 index 0000000000..d674c0acfd --- /dev/null +++ b/server/middlewares/email-not-verified-notice.js @@ -0,0 +1,32 @@ +import dedent from 'dedent'; + +const ALLOWED_METHODS = ['GET']; +const EXCLUDED_PATHS = [ + '/api/flyers/findOne', + '/signout', + '/update-email' +]; + +export default function emailNotVerifiedNotice() { + return function(req, res, next) { + if ( + ALLOWED_METHODS.indexOf(req.method) !== -1 && + EXCLUDED_PATHS.indexOf(req.path) === -1 + ) { + const { user } = req; + if (user && (!user.email || user.email === '' || !user.emailVerified)) { + req.flash( + 'danger', + dedent` + New privacy laws now require that we have an email address where we can reach + you. Please verify your email address below and click the link we send you to + confirm. + ` + ); + res.redirect('/update-email'); + return next; + } + } + return next(); + }; +} diff --git a/server/utils/middleware.js b/server/utils/middleware.js index f38afbdb37..7371824190 100644 --- a/server/utils/middleware.js +++ b/server/utils/middleware.js @@ -32,7 +32,7 @@ export function ifNoUser401(req, res, next) { return res.status(401).end(); } -export function ifNotVerifiedRedirectToSettings(req, res, next) { +export function ifNotVerifiedRedirectToUpdateEmail(req, res, next) { const { user } = req; if (!user) { return next(); diff --git a/server/views/account/update-email.jade b/server/views/account/update-email.jade new file mode 100644 index 0000000000..53071a0747 --- /dev/null +++ b/server/views/account/update-email.jade @@ -0,0 +1,57 @@ +extends ../layout +block content + .container + .row.flashMessage.negative-30 + .col-xs-12 + #flash-board.alert.fade.in(style='display: none;') + button.close(type='button', data-dismiss='alert') + span.ion-close-circled#flash-close + #flash-content + h2.text-center Update your email address here: + form.form-horizontal.update-email(method='POST', action='/api/users/#{user.id}/update-email', name="updateEmailForm") + .row + .col-sm-6.col-sm-offset-3 + input(type='hidden', name='_csrf', value=_csrf) + .form-group + input.input-lg.form-control(type='email', name='email', id='email', value=user.email || '', placeholder=user.email || 'Enter your new email', autofocus, required, autocomplete="off") + .form-group + button.btn.btn-lg.btn-primary.btn-block(type='submit')= !user.email || user.emailVerified ? 'Update my Email' : 'Verify Email' + a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/signout') + | Sign out + + script. + $(document).ready(function() { + $('form').submit(function(event){ + event.preventDefault(); + $('#flash-board').hide(); + var $form = $(event.target); + $.ajax({ + type : 'POST', + url : $form.attr('action'), + data : $form.serialize(), + dataType : 'json', + encode : true, + xhrFields : { withCredentials: true } + }) + .fail(error => { + if (error.responseText){ + var data = JSON.parse(error.responseText); + if(data.message) + $('#flash-content').html(data.message); + $('#flash-board') + .removeClass('alert-success') + .addClass('alert-info') + .fadeIn(); + } + }) + .done(data =>{ + if(data && data.message){ + $('#flash-content').html(data.message); + $('#flash-board') + .removeClass('alert-info') + .addClass('alert-success') + .fadeIn(); + } + }); + }); + }); diff --git a/server/views/partials/navbar.jade b/server/views/partials/navbar.jade index a866c4ee57..062d2369d0 100644 --- a/server/views/partials/navbar.jade +++ b/server/views/partials/navbar.jade @@ -14,7 +14,7 @@ nav.navbar.navbar-default.navbar-static-top.nav-height a(href='https://forum.freecodecamp.org', target='_blank' rel='noopener') Forum if !user li - a(href='/signin') Start Coding + a(href='/signin') Sign in else li a(href='/settings') Settings From 12b2c556ec8c6f8792bc65d72a916d24522e66b9 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Sat, 26 May 2018 00:31:21 +0530 Subject: [PATCH 02/10] fix(views): update static email update views --- server/boot/authentication.js | 8 -------- server/middlewares/email-not-verified-notice.js | 4 ++-- server/views/account/update-email.jade | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/server/boot/authentication.js b/server/boot/authentication.js index cf108fa3ba..cd9db97b8d 100644 --- a/server/boot/authentication.js +++ b/server/boot/authentication.js @@ -38,14 +38,6 @@ module.exports = function enableAuthentication(app) { ifUserRedirect, (req, res) => res.redirect(301, '/auth/auth0')); - router.get( - '/update-email', - ifNoUserRedirectHome, - (req, res) => res.render('account/update-email', { - title: 'Update your email' - }) - ); - router.get('/signout', (req, res) => { req.logout(); req.session.destroy( (err) => { diff --git a/server/middlewares/email-not-verified-notice.js b/server/middlewares/email-not-verified-notice.js index d674c0acfd..bdc430c433 100644 --- a/server/middlewares/email-not-verified-notice.js +++ b/server/middlewares/email-not-verified-notice.js @@ -4,7 +4,7 @@ const ALLOWED_METHODS = ['GET']; const EXCLUDED_PATHS = [ '/api/flyers/findOne', '/signout', - '/update-email' + '/settings/update-email' ]; export default function emailNotVerifiedNotice() { @@ -23,7 +23,7 @@ export default function emailNotVerifiedNotice() { confirm. ` ); - res.redirect('/update-email'); + res.redirect('/settings/update-email'); return next; } } diff --git a/server/views/account/update-email.jade b/server/views/account/update-email.jade index 53071a0747..c1887bed74 100644 --- a/server/views/account/update-email.jade +++ b/server/views/account/update-email.jade @@ -8,7 +8,7 @@ block content span.ion-close-circled#flash-close #flash-content h2.text-center Update your email address here: - form.form-horizontal.update-email(method='POST', action='/api/users/#{user.id}/update-email', name="updateEmailForm") + form.form-horizontal.update-email(method='POST', action='/update-my-email', name="updateEmailForm") .row .col-sm-6.col-sm-offset-3 input(type='hidden', name='_csrf', value=_csrf) From 9cf1d67e024f9993fd8d7b2aec46af0111bf8b5a Mon Sep 17 00:00:00 2001 From: Stuart Taylor Date: Sat, 26 May 2018 12:54:54 +0100 Subject: [PATCH 03/10] fix(Observables): Remove observables from requestUpdateEmail method --- common/models/user.js | 112 ++++++++---------- server/boot/authentication.js | 8 ++ server/boot/settings.js | 8 +- .../middlewares/email-not-verified-notice.js | 5 +- 4 files changed, 61 insertions(+), 72 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 56acb92ac5..fa61aca7ab 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -587,82 +587,66 @@ module.exports = function(User) { User.prototype.requestUpdateEmail = function requestUpdateEmail(newEmail) { const currentEmail = this.email; - return Observable.defer(() => { - const isOwnEmail = isTheSame(newEmail, currentEmail); - const sameUpdate = isTheSame(newEmail, this.newEmail); - const messageOrNull = getWaitMessage(this.emailVerifyTTL); - if (isOwnEmail) { - if (this.emailVerified) { - // email is already associated and verified with this account - throw wrapHandledError( - new Error('email is already verified'), - { - type: 'info', - message: `${newEmail} is already associated with this account.` - } - ); - } else if (!this.emailVerified && messageOrNull) { - // email is associated but unverified and - // email is within time limit - throw wrapHandledError( - new Error(), - { - type: 'info', - message: messageOrNull - } - ); - } - } - if (sameUpdate && messageOrNull) { - // trying to update with the same newEmail and - // confirmation email is still valid + const isOwnEmail = isTheSame(newEmail, currentEmail); + const sameUpdate = isTheSame(newEmail, this.newEmail); + const messageOrNull = getWaitMessage(this.emailVerifyTTL); + if (isOwnEmail) { + if (this.emailVerified) { + // email is already associated and verified with this account throw wrapHandledError( - new Error(), + new Error('email is already verified'), { type: 'info', - message: dedent` - We have already sent an email confirmation request to ${newEmail}. - Please check your inbox.` + message: `${newEmail} is already associated with this account.` } ); - } - if (!isEmail('' + newEmail)) { - throw createEmailError(); - } - // newEmail is not associated with this user, and - // this attempt to change email is the first or - // previous attempts have expired - return Observable.if( - () => isOwnEmail || (sameUpdate && messageOrNull), - Observable.empty(), - // defer prevents the promise from firing prematurely (before subscribe) - Observable.defer(() => User.doesExist(null, newEmail)) - ) - .do(exists => { - if (exists) { - // newEmail is not associated with this account, - // but is associated with different account + } else if (!this.emailVerified && messageOrNull) { + // email is associated but unverified and + // email is within time limit throw wrapHandledError( - new Error('email already in use'), + new Error(), { type: 'info', - message: - `${newEmail} is already associated with another account.` + message: messageOrNull } ); } + } + if (sameUpdate && messageOrNull) { + // trying to update with the same newEmail and + // confirmation email is still valid + throw wrapHandledError( + new Error(), + { + type: 'info', + message: dedent` + We have already sent an email confirmation request to ${newEmail}. + Please check your inbox.` + } + ); + } + if (!isEmail('' + newEmail)) { + throw createEmailError(); + } + // newEmail is not associated with this user, and + // this attempt to change email is the first or + // previous attempts have expired + + if (isOwnEmail || (sameUpdate && !messageOrNull)) { + const update = { + newEmail, + emailVerified: false, + emailVerifyTTL: new Date() + }; + return this.update$(update).toPromise() + .then(() => { + Object.assign(this, update); + return; }) - .flatMap(() => { - const update = { - newEmail, - emailVerified: false, - emailVerifyTTL: new Date() - }; - return this.update$(update) - .do(() => Object.assign(this, update)) - .flatMap(() => this.requestAuthEmail(false, newEmail)); - }); - }); + .then(() => this.requestAuthEmail(false, newEmail).toPromise()); + } else { + return 'Something unexpected happened whilst updating your email.'; + } }; function requestCompletedChallenges() { diff --git a/server/boot/authentication.js b/server/boot/authentication.js index cd9db97b8d..cf108fa3ba 100644 --- a/server/boot/authentication.js +++ b/server/boot/authentication.js @@ -38,6 +38,14 @@ module.exports = function enableAuthentication(app) { ifUserRedirect, (req, res) => res.redirect(301, '/auth/auth0')); + router.get( + '/update-email', + ifNoUserRedirectHome, + (req, res) => res.render('account/update-email', { + title: 'Update your email' + }) + ); + router.get('/signout', (req, res) => { req.logout(); req.session.destroy( (err) => { diff --git a/server/boot/settings.js b/server/boot/settings.js index f2d67628f4..4ccfa2944b 100644 --- a/server/boot/settings.js +++ b/server/boot/settings.js @@ -37,13 +37,9 @@ export default function settingsController(app) { .withMessage('Email format is invalid.') ]; - function updateMyEmail(req, res, next) { + function updateMyEmail(req, res) { const { user, body: { email } } = req; - return user.requestUpdateEmail(email) - .subscribe( - message => res.json({ message }), - next - ); + return res.json({ message: user.requestUpdateEmail(email) } ); } const updateMyCurrentChallengeValidators = [ diff --git a/server/middlewares/email-not-verified-notice.js b/server/middlewares/email-not-verified-notice.js index bdc430c433..d3d6f5012d 100644 --- a/server/middlewares/email-not-verified-notice.js +++ b/server/middlewares/email-not-verified-notice.js @@ -4,7 +4,8 @@ const ALLOWED_METHODS = ['GET']; const EXCLUDED_PATHS = [ '/api/flyers/findOne', '/signout', - '/settings/update-email' + '/update-email', + '/passwordless-change' ]; export default function emailNotVerifiedNotice() { @@ -23,7 +24,7 @@ export default function emailNotVerifiedNotice() { confirm. ` ); - res.redirect('/settings/update-email'); + res.redirect('/update-email'); return next; } } From eebe4036ece50aab988d2d0bc9123ab6da7fd5e9 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Sat, 26 May 2018 18:28:20 +0530 Subject: [PATCH 04/10] fix: remove cookies when signout, logout (remote) and delete (remote) --- common/models/user.js | 13 +++++++++---- server/boot/authentication.js | 12 ++++++++---- server/boot/user.js | 12 ++++++++---- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index fa61aca7ab..032a5d3d41 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -407,10 +407,15 @@ module.exports = function(User) { ); }; - User.afterRemote('logout', function(ctx, result, next) { - var res = ctx.res; - res.clearCookie('access_token'); - res.clearCookie('userId'); + User.afterRemote('logout', function({req, res}, result, next) { + const config = { + signed: !!req.signedCookies, + domain: process.env.COOKIE_DOMAIN || 'localhost' + }; + res.clearCookie('jwt_access_token', config); + res.clearCookie('access_token', config); + res.clearCookie('userId', config); + res.clearCookie('_csrf', config); next(); }); diff --git a/server/boot/authentication.js b/server/boot/authentication.js index cf108fa3ba..1e8e576b6e 100644 --- a/server/boot/authentication.js +++ b/server/boot/authentication.js @@ -59,10 +59,14 @@ module.exports = function enableAuthentication(app) { } ); } - res.clearCookie('jwt_access_token'); - res.clearCookie('access_token'); - res.clearCookie('userId'); - res.clearCookie('_csrf'); + const config = { + signed: !!req.signedCookies, + domain: process.env.COOKIE_DOMAIN || 'localhost' + }; + res.clearCookie('jwt_access_token', config); + res.clearCookie('access_token', config); + res.clearCookie('userId', config); + res.clearCookie('_csrf', config); res.redirect('/'); }); }); diff --git a/server/boot/user.js b/server/boot/user.js index bc05d88470..530114e5df 100644 --- a/server/boot/user.js +++ b/server/boot/user.js @@ -119,10 +119,14 @@ module.exports = function(app) { if (err) { return next(err); } req.logout(); req.flash('success', 'You have successfully deleted your account.'); - res.clearCookie('jwt_access_token'); - res.clearCookie('access_token'); - res.clearCookie('userId'); - res.clearCookie('_csrf'); + const config = { + signed: !!req.signedCookies, + domain: process.env.COOKIE_DOMAIN || 'localhost' + }; + res.clearCookie('jwt_access_token', config); + res.clearCookie('access_token', config); + res.clearCookie('userId', config); + res.clearCookie('_csrf', config); return res.status(200).end(); }); } From d3567fbb9bc3171c7a98eb0b428468bc516d09e5 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Sun, 27 May 2018 19:41:58 +0530 Subject: [PATCH 05/10] fix: add route for lear redirects --- server/middlewares/email-not-verified-notice.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/middlewares/email-not-verified-notice.js b/server/middlewares/email-not-verified-notice.js index d3d6f5012d..6a4c110f5c 100644 --- a/server/middlewares/email-not-verified-notice.js +++ b/server/middlewares/email-not-verified-notice.js @@ -5,7 +5,8 @@ const EXCLUDED_PATHS = [ '/api/flyers/findOne', '/signout', '/update-email', - '/passwordless-change' + '/passwordless-change', + '/external/services/user' ]; export default function emailNotVerifiedNotice() { From f52d5b536901b5e9136556afb56e2c710aec7568 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Mon, 28 May 2018 18:47:36 +0530 Subject: [PATCH 06/10] fix: resolve the query back to the promise --- common/models/user.js | 64 ++++++++++++++++++++--------------------- server/boot/settings.js | 8 ++++-- 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 032a5d3d41..39080d1ff4 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -591,33 +591,26 @@ module.exports = function(User) { User.prototype.requestAuthEmail = requestAuthEmail; User.prototype.requestUpdateEmail = function requestUpdateEmail(newEmail) { + const currentEmail = this.email; const isOwnEmail = isTheSame(newEmail, currentEmail); - const sameUpdate = isTheSame(newEmail, this.newEmail); - const messageOrNull = getWaitMessage(this.emailVerifyTTL); - if (isOwnEmail) { - if (this.emailVerified) { - // email is already associated and verified with this account - throw wrapHandledError( - new Error('email is already verified'), - { - type: 'info', - message: `${newEmail} is already associated with this account.` - } - ); - } else if (!this.emailVerified && messageOrNull) { - // email is associated but unverified and - // email is within time limit - throw wrapHandledError( - new Error(), - { - type: 'info', - message: messageOrNull - } - ); + const isResendUpdateToSameEmail = isTheSame(newEmail, this.newEmail); + const isLinkSentWithinLimit = getWaitMessage(this.emailVerifyTTL); + const isVerifiedEmail = this.emailVerified; + + if (isOwnEmail && isVerifiedEmail) { + // email is already associated and verified with this account + throw wrapHandledError( + new Error('email is already verified'), + { + type: 'info', + message: ` + ${newEmail} is already associated with this account. + You can update a new email address instead.` } + ); } - if (sameUpdate && messageOrNull) { + if (isResendUpdateToSameEmail && isLinkSentWithinLimit) { // trying to update with the same newEmail and // confirmation email is still valid throw wrapHandledError( @@ -626,29 +619,34 @@ module.exports = function(User) { type: 'info', message: dedent` We have already sent an email confirmation request to ${newEmail}. - Please check your inbox.` + ${isLinkSentWithinLimit}` } ); } if (!isEmail('' + newEmail)) { throw createEmailError(); } + // newEmail is not associated with this user, and // this attempt to change email is the first or // previous attempts have expired - - if (isOwnEmail || (sameUpdate && !messageOrNull)) { - const update = { + if ( + !isOwnEmail || + (isOwnEmail && !isVerifiedEmail) || + (isResendUpdateToSameEmail && !isLinkSentWithinLimit) + ) { + const updateConfig = { newEmail, emailVerified: false, emailVerifyTTL: new Date() }; - return this.update$(update).toPromise() - .then(() => { - Object.assign(this, update); - return; - }) - .then(() => this.requestAuthEmail(false, newEmail).toPromise()); + return Observable.forkJoin( + this.update$(updateConfig), + this.requestAuthEmail(false, newEmail), + (user, message) => ({ user, message }) + ) + .map(({ message }) => message); + } else { return 'Something unexpected happened whilst updating your email.'; } diff --git a/server/boot/settings.js b/server/boot/settings.js index 4ccfa2944b..e240dd2e48 100644 --- a/server/boot/settings.js +++ b/server/boot/settings.js @@ -37,9 +37,13 @@ export default function settingsController(app) { .withMessage('Email format is invalid.') ]; - function updateMyEmail(req, res) { + function updateMyEmail(req, res, next) { const { user, body: { email } } = req; - return res.json({ message: user.requestUpdateEmail(email) } ); + return user.requestUpdateEmail(email) + .subscribe( + (message) => res.json({ message: message }), + next + ); } const updateMyCurrentChallengeValidators = [ From 924cf5ae49ba9e5e0e64d7d43a5f3d6956e023d9 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Mon, 28 May 2018 19:23:17 +0530 Subject: [PATCH 07/10] fix(user): check if email is someone else --- common/models/user.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 39080d1ff4..8ffacd9d5c 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -640,12 +640,31 @@ module.exports = function(User) { emailVerified: false, emailVerifyTTL: new Date() }; - return Observable.forkJoin( - this.update$(updateConfig), - this.requestAuthEmail(false, newEmail), - (user, message) => ({ user, message }) - ) - .map(({ message }) => message); + + // defer prevents the promise from firing prematurely (before subscribe) + return Observable.defer(() => User.doesExist(null, newEmail)) + .do(exists => { + if (exists && !isOwnEmail) { + // newEmail is not associated with this account, + // but is associated with different account + throw wrapHandledError( + new Error('email already in use'), + { + type: 'info', + message: + `${newEmail} is already associated with another account.` + } + ); + } + }) + .flatMap(()=>{ + return Observable.forkJoin( + this.update$(updateConfig), + this.requestAuthEmail(false, newEmail), + (user, message) => ({ user, message }) + ) + .map(({ message }) => message); + }); } else { return 'Something unexpected happened whilst updating your email.'; From 5f8eb3615a57d423e62d75ddfaae626892505baf Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Tue, 29 May 2018 01:59:37 +0530 Subject: [PATCH 08/10] fix: update middleware passthru and user in memory --- common/models/user.js | 6 ++++-- server/boot/settings.js | 2 +- server/middlewares/email-not-verified-notice.js | 1 + server/middlewares/privacy-terms-notice.js | 5 ++++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 8ffacd9d5c..b33b40cdb2 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -661,9 +661,11 @@ module.exports = function(User) { return Observable.forkJoin( this.update$(updateConfig), this.requestAuthEmail(false, newEmail), - (user, message) => ({ user, message }) + (_, message) => message ) - .map(({ message }) => message); + .do(() => { + Object.assign(this, updateConfig); + }); }); } else { diff --git a/server/boot/settings.js b/server/boot/settings.js index e240dd2e48..f2d67628f4 100644 --- a/server/boot/settings.js +++ b/server/boot/settings.js @@ -41,7 +41,7 @@ export default function settingsController(app) { const { user, body: { email } } = req; return user.requestUpdateEmail(email) .subscribe( - (message) => res.json({ message: message }), + message => res.json({ message }), next ); } diff --git a/server/middlewares/email-not-verified-notice.js b/server/middlewares/email-not-verified-notice.js index 6a4c110f5c..0c53d14797 100644 --- a/server/middlewares/email-not-verified-notice.js +++ b/server/middlewares/email-not-verified-notice.js @@ -4,6 +4,7 @@ const ALLOWED_METHODS = ['GET']; const EXCLUDED_PATHS = [ '/api/flyers/findOne', '/signout', + '/accept-privacy-terms', '/update-email', '/passwordless-change', '/external/services/user' diff --git a/server/middlewares/privacy-terms-notice.js b/server/middlewares/privacy-terms-notice.js index 1dc892a7be..ea2a5316ed 100644 --- a/server/middlewares/privacy-terms-notice.js +++ b/server/middlewares/privacy-terms-notice.js @@ -2,7 +2,10 @@ const ALLOWED_METHODS = ['GET']; const EXCLUDED_PATHS = [ '/api/flyers/findOne', '/signout', - '/accept-privacy-terms' + '/accept-privacy-terms', + '/update-email', + '/passwordless-change', + '/external/services/user' ]; export default function privacyTermsNotAcceptedNotice() { From c0156b41e2f879928bee12a0a1fb78cd637db545 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Tue, 29 May 2018 02:23:19 +0530 Subject: [PATCH 09/10] fix: make success message relevant --- common/models/user.js | 9 +- .../views/account/accept-privacy-terms.jade | 14 +-- server/views/account/update-email.jade | 98 +++++++++---------- 3 files changed, 58 insertions(+), 63 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index b33b40cdb2..69472224ff 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -576,14 +576,9 @@ module.exports = function(User) { this.update$({ emailAuthLinkTTL }) ); }) - .map(() => isSignUp ? + .map(() => dedent` - We created a new account for you! - Check your email and click the sign in link we sent you. - ` : - dedent` - We found your existing account. - Check your email and click the sign in link we sent you. + Check your email and click the link we sent you to confirm you email. ` ); } diff --git a/server/views/account/accept-privacy-terms.jade b/server/views/account/accept-privacy-terms.jade index f0b9f0ef63..eaf7c7c804 100644 --- a/server/views/account/accept-privacy-terms.jade +++ b/server/views/account/accept-privacy-terms.jade @@ -1,12 +1,12 @@ extends ../layout block content - .container - .row.flashMessage.negative-30 - .col-sm-6.col-sm-offset-3 - #flash-board.alert.fade.in(style='display: none;') - button.close(type='button', data-dismiss='alert') - span.ion-close-circled#flash-close - #flash-content + .row.flashMessage.negative-30 + .col-xs-12.col-sm-8.col-sm-offset-2.col-md-6.col-md-offset-3 + #flash-board.alert.fade.in(style='display: none;') + button.close(type='button', data-dismiss='alert') + span.ion-close-circled#flash-close + #flash-content + .row .col-xs-12 #accept-privacy-terms .row diff --git a/server/views/account/update-email.jade b/server/views/account/update-email.jade index c1887bed74..0de7f0183d 100644 --- a/server/views/account/update-email.jade +++ b/server/views/account/update-email.jade @@ -1,57 +1,57 @@ extends ../layout block content + .row.flashMessage.negative-30 + .col-xs-12.col-sm-8.col-sm-offset-2.col-md-6.col-md-offset-3 + #flash-board.alert.fade.in(style='display: none;') + button.close(type='button', data-dismiss='alert') + span.ion-close-circled#flash-close + #flash-content .container - .row.flashMessage.negative-30 - .col-xs-12 - #flash-board.alert.fade.in(style='display: none;') - button.close(type='button', data-dismiss='alert') - span.ion-close-circled#flash-close - #flash-content - h2.text-center Update your email address here: - form.form-horizontal.update-email(method='POST', action='/update-my-email', name="updateEmailForm") - .row - .col-sm-6.col-sm-offset-3 - input(type='hidden', name='_csrf', value=_csrf) - .form-group - input.input-lg.form-control(type='email', name='email', id='email', value=user.email || '', placeholder=user.email || 'Enter your new email', autofocus, required, autocomplete="off") - .form-group - button.btn.btn-lg.btn-primary.btn-block(type='submit')= !user.email || user.emailVerified ? 'Update my Email' : 'Verify Email' - a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/signout') - | Sign out + h2.text-center Update your email address here: + form.form-horizontal.update-email(method='POST', action='/update-my-email', name="updateEmailForm") + .row + .col-sm-6.col-sm-offset-3 + input(type='hidden', name='_csrf', value=_csrf) + .form-group + input.input-lg.form-control(type='email', name='email', id='email', value=user.email || '', placeholder=user.email || 'Enter your new email', autofocus, required, autocomplete="off") + .form-group + button.btn.btn-lg.btn-primary.btn-block(type='submit')= !user.email || user.emailVerified ? 'Update my Email' : 'Verify Email' + a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/signout') + | Sign out - script. - $(document).ready(function() { - $('form').submit(function(event){ - event.preventDefault(); - $('#flash-board').hide(); - var $form = $(event.target); - $.ajax({ - type : 'POST', - url : $form.attr('action'), - data : $form.serialize(), - dataType : 'json', - encode : true, - xhrFields : { withCredentials: true } - }) - .fail(error => { - if (error.responseText){ - var data = JSON.parse(error.responseText); - if(data.message) + script. + $(document).ready(function() { + $('form').submit(function(event){ + event.preventDefault(); + $('#flash-board').hide(); + var $form = $(event.target); + $.ajax({ + type : 'POST', + url : $form.attr('action'), + data : $form.serialize(), + dataType : 'json', + encode : true, + xhrFields : { withCredentials: true } + }) + .fail(error => { + if (error.responseText){ + var data = JSON.parse(error.responseText); + if(data.message) + $('#flash-content').html(data.message); + $('#flash-board') + .removeClass('alert-success') + .addClass('alert-info') + .fadeIn(); + } + }) + .done(data =>{ + if(data && data.message){ $('#flash-content').html(data.message); $('#flash-board') - .removeClass('alert-success') - .addClass('alert-info') - .fadeIn(); + .removeClass('alert-info') + .addClass('alert-success') + .fadeIn(); } - }) - .done(data =>{ - if(data && data.message){ - $('#flash-content').html(data.message); - $('#flash-board') - .removeClass('alert-info') - .addClass('alert-success') - .fadeIn(); - } + }); }); - }); - }); + }); From 1af24d131ec772e9d1da4b186e421162830bfc75 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Tue, 29 May 2018 02:54:27 +0530 Subject: [PATCH 10/10] fix: always redirect to home on login --- server/boot/authentication.js | 12 +----------- server/component-passport.js | 3 +-- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/server/boot/authentication.js b/server/boot/authentication.js index 1e8e576b6e..2c874382e1 100644 --- a/server/boot/authentication.js +++ b/server/boot/authentication.js @@ -193,21 +193,11 @@ module.exports = function enableAuthentication(app) { // update user and log them in .map(user => user.loginByRequest(req, res)) .do(() => { - let redirectTo = '/'; - - if ( - req.session && - req.session.returnTo - ) { - redirectTo = req.session.returnTo; - } - req.flash( 'success', 'Success! You have signed in to your account. Happy Coding!' ); - - return res.redirect(redirectTo); + return res.redirect('/'); }) .subscribe( () => {}, diff --git a/server/component-passport.js b/server/component-passport.js index 7e76b97f08..71790ba89a 100644 --- a/server/component-passport.js +++ b/server/component-passport.js @@ -106,9 +106,8 @@ export default function setupPassport(app) { // https://stackoverflow.com/q/37430452 let successRedirect = (req) => { if (!!req && req.session && req.session.returnTo) { - let returnTo = req.session.returnTo; delete req.session.returnTo; - return returnTo; + return '/'; } return config.successRedirect || ''; };