From 3f332fc1f0f83efde2cb26d9b5c496b0f2de924e Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Mon, 24 Apr 2017 00:37:10 +0530 Subject: [PATCH] fix: Add error handling and refactoring of Observable methods --- common/models/user.js | 54 ++++++++++++++++++++----------------------- server/boot/user.js | 6 +---- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index b7a3ceb81a..5e68250197 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -107,7 +107,10 @@ module.exports = function(User) { User.findOne$ = Observable.fromNodeCallback(User.findOne, User); User.update$ = Observable.fromNodeCallback(User.updateAll, User); User.count$ = Observable.fromNodeCallback(User.count, User); - User.findOrCreate$ = Observable.fromCallback(User.findOrCreate, User); + User.findOrCreate$ = Observable.fromNodeCallback(User.findOrCreate, User); + User.prototype.createAccessToken$ = Observable.fromNodeCallback( + User.prototype.createAccessToken + ); }); User.beforeRemote('create', function({ req }) { @@ -566,30 +569,25 @@ module.exports = function(User) { emailVerified: false }; return User.findOrCreate$({ where: { email }}, userObj) - .map(([ err, user, isCreated ]) => { - if (err) { - return dedent` - Oops, something is not right, please try again later. - `; - } + .flatMap(([ user, isCreated ]) => { const minutesLeft = getWaitPeriod(user.emailAuthLinkTTL); - if (minutesLeft) { + if (minutesLeft > 0) { const timeToWait = minutesLeft ? `${minutesLeft} minute${minutesLeft > 1 ? 's' : ''}` : 'a few seconds'; debug('request before wait time : ' + timeToWait); - return dedent` + return Observable.of(dedent` Please wait ${timeToWait} to resend an authentication link. - `; + `); } const renderAuthEmail = isCreated ? renderSignUpEmail : renderSignInEmail; - // create a temporary access token with ttl for 1 hour - return user.createAccessToken({ ttl: 60 * 60 * 1000 }, (err, token) => { - if (err) { throw err; } + // create a temporary access token with ttl for 15 minutes + return user.createAccessToken$({ ttl: 15 * 60 * 1000 }) + .flatMap(token => { const { id: loginToken } = token; const loginEmail = user.email; @@ -597,29 +595,27 @@ module.exports = function(User) { const mailOptions = { type: 'email', to: user.email, -<<<<<<< HEAD - from: 'Team@freecodecamp.com', - subject: 'Free Code Camp - Authentication Request!', -======= from: isDev ? process.env.EMAIL_SENDER : 'team@freecodecamp.com', subject: 'freeCodeCamp - Authentication Request!', ->>>>>>> fix(email): allow sender email var for development text: renderAuthEmail({ loginEmail, loginToken }) }; - this.email.send(mailOptions, err =>{ - if (err) { throw err; } - }); - const emailAuthLinkTTL = token.created; - this.update$({ - emailAuthLinkTTL - }) - .do(() => { - this.emailAuthLinkTTL = emailAuthLinkTTL; + return this.email.send$(mailOptions) + .flatMap(() => { + const emailAuthLinkTTL = token.created; + return this.update$({ + emailAuthLinkTTL + }) + .map(() => { + return dedent` + If you entered a valid email, a magic link is on its way. + Please follow that link to sign in. + `; + }); }); return dedent` @@ -628,8 +624,8 @@ module.exports = function(User) { `; }); }) - .map((msg) => { - if (msg) { return msg; } + .catch(err => { + if (err) { debug(err); } return dedent` Oops, something is not right, please try again later. `; diff --git a/server/boot/user.js b/server/boot/user.js index 9d1f16c39b..b0e108510e 100644 --- a/server/boot/user.js +++ b/server/boot/user.js @@ -290,15 +290,11 @@ module.exports = function(app) { const userId = authToken.userId; return User.findById(userId, (err, user) => { - if (err) { + if (err || !user || user.email !== authEmailId) { debug(err); req.flash('info', { msg: defaultErrorMsg }); return res.redirect('/email-signin'); } - if (user.email !== authEmailId) { - req.flash('info', { msg: defaultErrorMsg }); - return res.redirect('/email-signin'); - } return authToken.validate((err, isValid) => { if (err) { throw err; } if (!isValid) {