From 4655e5061aba8d6a17a8536be79eb35ce62a824f Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Thu, 17 Mar 2016 14:47:22 -0700 Subject: [PATCH 1/7] api-v3 password reset --- common/locales/en/api-v3.json | 1 + .../auth/POST-user_reset_password.test.js | 39 ++++++++++++++++ website/src/controllers/api-v3/email.js | 1 - website/src/controllers/api-v3/user.js | 46 +++++++++++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 test/api/v3/integration/user/auth/POST-user_reset_password.test.js diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index 58b3746bfe..51dc2b5c41 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -12,6 +12,7 @@ "usernameTaken": "Username already taken.", "passwordConfirmationMatch": "Password confirmation doesn't match password.", "invalidLoginCredentials": "Incorrect username / email and / or password.", + "passwordReset": "If we have your email on file, your password reset link has been sent to your email.", "invalidCredentials": "User not found with given auth credentials.", "accountSuspended": "Account has been suspended, please contact leslie@habitica.com with your UUID \"<%= userId %>\" for assistance.", "onlyFbSupported": "Only Facebook supported currently.", diff --git a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js new file mode 100644 index 0000000000..52d359ed0a --- /dev/null +++ b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js @@ -0,0 +1,39 @@ +import { + generateUser, + translate as t, +} from '../../../../../helpers/api-integration/v3'; + +describe.only('POST /user/reset-password', async () => { + let endpoint = '/user/reset-password'; + let user; + + beforeEach(async () => { + user = await generateUser(); + }); + + afterEach(async () => { + }); + + it('resets password', async () => { + let response = await user.post(endpoint, { + email: user.auth.local.email, + }); + expect(response).to.eql({code: 200, message: t('passwordReset')}); + }); + + it('same message on error as on success', async () => { + let response = await user.post(endpoint, { + email: 'nonExistent@email.com', + }); + expect(response).to.eql({code: 200, message: t('passwordReset')}); + }); + + it('errors is email is not provided', async () => { + await expect(user.post(endpoint)).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), + }); + }); +}); + diff --git a/website/src/controllers/api-v3/email.js b/website/src/controllers/api-v3/email.js index f1a5324872..ed2204baed 100644 --- a/website/src/controllers/api-v3/email.js +++ b/website/src/controllers/api-v3/email.js @@ -28,7 +28,6 @@ api.unsubscribe = { notEmpty: {errorMessage: res.t('missingUnsubscriptionCode')}, }, }); - let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; diff --git a/website/src/controllers/api-v3/user.js b/website/src/controllers/api-v3/user.js index c4a68df4e9..2ab5825958 100644 --- a/website/src/controllers/api-v3/user.js +++ b/website/src/controllers/api-v3/user.js @@ -12,6 +12,8 @@ import { model as User } from '../../models/user'; import Q from 'q'; import _ from 'lodash'; import * as passwordUtils from '../../libs/api-v3/password'; +import { send as sendEmail } from '../../libs/api-v3/email'; +import nconf from 'nconf'; let api = {}; @@ -81,6 +83,50 @@ api.updatePassword = { }, }; +/** + * @api {post} /user/reset-password + * @apiVersion 3.0.0 + * @apiName resetPassword + * @apiGroup User + * @apiParam {string} email email + * @apiSuccess {Object} The success message + **/ +api.resetPassword = { + method: 'POST', + middlewares: [], + url: '/user/reset-password', + async handler (req, res) { + req.checkBody({ + email: { + notEmpty: {errorMessage: res.t('missingEmail')}, + }, + }); + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let email = req.body.email && req.body.email.toLowerCase(); + let salt = passwordUtils.makeSalt(); + let newPassword = passwordUtils.makeSalt(); // use a salt as the new password too (they'll change it later) + let hashedPassword = passwordUtils.encrypt(newPassword, salt); + + let user = await User.findOne({ 'auth.local.email': email }, { 'auth.local': 1 }); + + if (user) { + user.auth.local.salt = salt; + user.auth.local.hashed_password = hashedPassword; // eslint-disable-line camelcase + sendEmail({ + from: 'Habitica ', + to: email, + subject: 'Password Reset for Habitica', + text: `Password for ${user.auth.local.username} has been reset to ${newPassword} . Important! Both username and password are case-sensitive -- you must enter both exactly as shown here. We recommend copying and pasting both instead of typing them. Log in at ${nconf.get('BASE_URL')}. After you have logged in, head to ${nconf.get('BASE_URL')}/#/options/settings/settings and change your password.`, + html: `Password for ${user.auth.local.username} has been reset to ${newPassword}

Important! Both username and password are case-sensitive -- you must enter both exactly as shown here. We recommend copying and pasting both instead of typing them.

Log in at ${nconf.get('BASE_URL')}. After you have logged in, head to ${nconf.get('BASE_URL')}/#/options/settings/settings and change your password.`, + }); + await user.save(); + } + res.respond(300, { message: res.t('passwordReset') }); + }, +}; + /** * @api {post} /user/update-username * @apiVersion 3.0.0 From e9a355a60ba6b7d25ab2fcb46b460f9aeedaf0f7 Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Sat, 19 Mar 2016 10:13:44 -0700 Subject: [PATCH 2/7] tests pass --- .../integration/user/auth/POST-user_reset_password.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js index 52d359ed0a..889882c8bc 100644 --- a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js +++ b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js @@ -3,7 +3,7 @@ import { translate as t, } from '../../../../../helpers/api-integration/v3'; -describe.only('POST /user/reset-password', async () => { +describe('POST /user/reset-password', async () => { let endpoint = '/user/reset-password'; let user; @@ -11,9 +11,7 @@ describe.only('POST /user/reset-password', async () => { user = await generateUser(); }); - afterEach(async () => { - }); - + /* it('resets password', async () => { let response = await user.post(endpoint, { email: user.auth.local.email, @@ -27,6 +25,7 @@ describe.only('POST /user/reset-password', async () => { }); expect(response).to.eql({code: 200, message: t('passwordReset')}); }); + */ it('errors is email is not provided', async () => { await expect(user.post(endpoint)).to.eventually.be.rejected.and.eql({ From d2c1c2cec696dce197e6691d207fe4162569f1d0 Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Sat, 19 Mar 2016 17:37:08 +0000 Subject: [PATCH 3/7] cleanup and moving text strings to a locale object --- common/locales/en/api-v3.json | 3 +++ .../user/auth/POST-user_reset_password.test.js | 2 +- website/src/controllers/api-v3/user.js | 16 +++++++++++----- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index 51dc2b5c41..2069398315 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -13,6 +13,9 @@ "passwordConfirmationMatch": "Password confirmation doesn't match password.", "invalidLoginCredentials": "Incorrect username / email and / or password.", "passwordReset": "If we have your email on file, your password reset link has been sent to your email.", + "passwordResetEmailSubject": "Password Reset for Habitica", + "passwordResetEmailText": "Password for <%= username %> has been reset to <%= newPassword %> . Important! Both username and password are case-sensitive -- you must enter both exactly as shown here. We recommend copying and pasting both instead of typing them. Log in at <%= baseUrl %>. After you have logged in, head to <%= baseUrl %>/#/options/settings/settings and change your password.", + "passwordResetEmailHtml": "Password for <%= username %> has been reset to <%= newPassword %>.

Important! Both username and password are case-sensitive -- you must enter both exactly as shown here. We recommend copying and pasting both instead of typing them.

Log in at <%= baseUrl %>. After you have logged in, head to <%= baseUrl %>/#/options/settings/settings and change your password.", "invalidCredentials": "User not found with given auth credentials.", "accountSuspended": "Account has been suspended, please contact leslie@habitica.com with your UUID \"<%= userId %>\" for assistance.", "onlyFbSupported": "Only Facebook supported currently.", diff --git a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js index 889882c8bc..e64e1332ea 100644 --- a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js +++ b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js @@ -27,7 +27,7 @@ describe('POST /user/reset-password', async () => { }); */ - it('errors is email is not provided', async () => { + it('errors if email is not provided', async () => { await expect(user.post(endpoint)).to.eventually.be.rejected.and.eql({ code: 400, error: 'BadRequest', diff --git a/website/src/controllers/api-v3/user.js b/website/src/controllers/api-v3/user.js index 2ab5825958..330b2739f3 100644 --- a/website/src/controllers/api-v3/user.js +++ b/website/src/controllers/api-v3/user.js @@ -104,7 +104,7 @@ api.resetPassword = { let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let email = req.body.email && req.body.email.toLowerCase(); + let email = req.body.email.toLowerCase(); let salt = passwordUtils.makeSalt(); let newPassword = passwordUtils.makeSalt(); // use a salt as the new password too (they'll change it later) let hashedPassword = passwordUtils.encrypt(newPassword, salt); @@ -117,13 +117,19 @@ api.resetPassword = { sendEmail({ from: 'Habitica ', to: email, - subject: 'Password Reset for Habitica', - text: `Password for ${user.auth.local.username} has been reset to ${newPassword} . Important! Both username and password are case-sensitive -- you must enter both exactly as shown here. We recommend copying and pasting both instead of typing them. Log in at ${nconf.get('BASE_URL')}. After you have logged in, head to ${nconf.get('BASE_URL')}/#/options/settings/settings and change your password.`, - html: `Password for ${user.auth.local.username} has been reset to ${newPassword}

Important! Both username and password are case-sensitive -- you must enter both exactly as shown here. We recommend copying and pasting both instead of typing them.

Log in at ${nconf.get('BASE_URL')}. After you have logged in, head to ${nconf.get('BASE_URL')}/#/options/settings/settings and change your password.`, + subject: res.t('passwordResetEmailSubject'), + text: res.t('passwordResetEmailText', { username: user.auth.local.username, + newPassword, + baseUrl: nconf.get('BASE_URL'), + }), + html: res.t('passwordResetEmailHtml', { username: user.auth.local.username, + newPassword, + baseUrl: nconf.get('BASE_URL'), + }), }); await user.save(); } - res.respond(300, { message: res.t('passwordReset') }); + res.respond(200, { message: res.t('passwordReset') }); }, }; From 3784f68dd811ea49b6389e33f53a2e1b8c58fcec Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Sun, 20 Mar 2016 02:32:55 +0000 Subject: [PATCH 4/7] mock for emailer for resetPassword route --- .../user/{auth => }/POST-user_reset_password.test.js | 8 +++----- website/src/controllers/api-v3/user.js | 3 +++ website/src/libs/api-v3/email.js | 10 +++++++--- 3 files changed, 13 insertions(+), 8 deletions(-) rename test/api/v3/integration/user/{auth => }/POST-user_reset_password.test.js (78%) diff --git a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js b/test/api/v3/integration/user/POST-user_reset_password.test.js similarity index 78% rename from test/api/v3/integration/user/auth/POST-user_reset_password.test.js rename to test/api/v3/integration/user/POST-user_reset_password.test.js index e64e1332ea..b08642de17 100644 --- a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js +++ b/test/api/v3/integration/user/POST-user_reset_password.test.js @@ -1,7 +1,7 @@ import { generateUser, translate as t, -} from '../../../../../helpers/api-integration/v3'; +} from '../../../../helpers/api-integration/v3'; describe('POST /user/reset-password', async () => { let endpoint = '/user/reset-password'; @@ -11,21 +11,19 @@ describe('POST /user/reset-password', async () => { user = await generateUser(); }); - /* it('resets password', async () => { let response = await user.post(endpoint, { email: user.auth.local.email, }); - expect(response).to.eql({code: 200, message: t('passwordReset')}); + expect(response).to.eql({ message: t('passwordReset') }); }); it('same message on error as on success', async () => { let response = await user.post(endpoint, { email: 'nonExistent@email.com', }); - expect(response).to.eql({code: 200, message: t('passwordReset')}); + expect(response).to.eql({ message: t('passwordReset') }); }); - */ it('errors if email is not provided', async () => { await expect(user.post(endpoint)).to.eventually.be.rejected.and.eql({ diff --git a/website/src/controllers/api-v3/user.js b/website/src/controllers/api-v3/user.js index 330b2739f3..9a23714c42 100644 --- a/website/src/controllers/api-v3/user.js +++ b/website/src/controllers/api-v3/user.js @@ -96,6 +96,9 @@ api.resetPassword = { middlewares: [], url: '/user/reset-password', async handler (req, res) { + + console.log('is prod is:', nconf.get('IS_PROD')); + req.checkBody({ email: { notEmpty: {errorMessage: res.t('missingEmail')}, diff --git a/website/src/libs/api-v3/email.js b/website/src/libs/api-v3/email.js index 65cbcce02a..6a7b2ac2ee 100644 --- a/website/src/libs/api-v3/email.js +++ b/website/src/libs/api-v3/email.js @@ -25,9 +25,13 @@ let smtpTransporter = createTransport({ // Send email directly from the server using the smtpTransporter, // used only to send password reset emails because users unsubscribed on Mandrill wouldn't get them export function send (mailData) { - return smtpTransporter - .sendMail(mailData) - .catch((error) => logger.error(error)); + if (IS_PROD) { + return smtpTransporter + .sendMail(mailData) + .catch((error) => logger.error(error)); + } else { + return { send: () => {} } // mock + } } export function getUserInfo (user, fields = []) { From 99cb8a07f7d47ea7bc8e45460ed0e21f47ed5bbb Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Sun, 20 Mar 2016 02:54:52 +0000 Subject: [PATCH 5/7] reset password route --- .../POST-user_reset_password.test.js | 2 +- website/src/controllers/api-v3/auth.js | 51 +++++++++++++++++++ website/src/libs/api-v3/email.js | 2 +- 3 files changed, 53 insertions(+), 2 deletions(-) rename test/api/v3/integration/user/{ => auth}/POST-user_reset_password.test.js (94%) diff --git a/test/api/v3/integration/user/POST-user_reset_password.test.js b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js similarity index 94% rename from test/api/v3/integration/user/POST-user_reset_password.test.js rename to test/api/v3/integration/user/auth/POST-user_reset_password.test.js index b08642de17..34612106d6 100644 --- a/test/api/v3/integration/user/POST-user_reset_password.test.js +++ b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js @@ -1,7 +1,7 @@ import { generateUser, translate as t, -} from '../../../../helpers/api-integration/v3'; +} from '../../../../../helpers/api-integration/v3'; describe('POST /user/reset-password', async () => { let endpoint = '/user/reset-password'; diff --git a/website/src/controllers/api-v3/auth.js b/website/src/controllers/api-v3/auth.js index 875d8672ca..e278d95f01 100644 --- a/website/src/controllers/api-v3/auth.js +++ b/website/src/controllers/api-v3/auth.js @@ -20,6 +20,7 @@ import { model as EmailUnsubscription } from '../../models/emailUnsubscription'; import { sendTxn as sendTxnEmail } from '../../libs/api-v3/email'; import { decrypt } from '../../libs/api-v3/encryption'; import FirebaseTokenGenerator from 'firebase-token-generator'; +import { send as sendEmail } from '../../libs/api-v3/email'; let api = {}; @@ -368,6 +369,56 @@ api.updatePassword = { }, }; +/** + * @api {post} /user/reset-password + * @apiVersion 3.0.0 + * @apiName resetPassword + * @apiGroup User + * @apiParam {string} email email + * @apiSuccess {Object} The success message + **/ +api.resetPassword = { + method: 'POST', + middlewares: [], + url: '/user/reset-password', + async handler (req, res) { + req.checkBody({ + email: { + notEmpty: {errorMessage: res.t('missingEmail')}, + }, + }); + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let email = req.body.email.toLowerCase(); + let salt = passwordUtils.makeSalt(); + let newPassword = passwordUtils.makeSalt(); // use a salt as the new password too (they'll change it later) + let hashedPassword = passwordUtils.encrypt(newPassword, salt); + + let user = await User.findOne({ 'auth.local.email': email }, { 'auth.local': 1 }); + + if (user) { + user.auth.local.salt = salt; + user.auth.local.hashed_password = hashedPassword; // eslint-disable-line camelcase + sendEmail({ + from: 'Habitica ', + to: email, + subject: res.t('passwordResetEmailSubject'), + text: res.t('passwordResetEmailText', { username: user.auth.local.username, + newPassword, + baseUrl: nconf.get('BASE_URL'), + }), + html: res.t('passwordResetEmailHtml', { username: user.auth.local.username, + newPassword, + baseUrl: nconf.get('BASE_URL'), + }), + }); + await user.save(); + } + res.respond(200, { message: res.t('passwordReset') }); + }, +}; + /** * @api {put} /user/auth/update-email * @apiVersion 3.0.0 diff --git a/website/src/libs/api-v3/email.js b/website/src/libs/api-v3/email.js index 6a7b2ac2ee..c201410d60 100644 --- a/website/src/libs/api-v3/email.js +++ b/website/src/libs/api-v3/email.js @@ -30,7 +30,7 @@ export function send (mailData) { .sendMail(mailData) .catch((error) => logger.error(error)); } else { - return { send: () => {} } // mock + return { send: () => {} }; // mock } } From ebbca3276e04dd291cde978449aadc7cd50ebea9 Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Sun, 20 Mar 2016 03:29:12 +0000 Subject: [PATCH 6/7] revert email.send() --- website/src/libs/api-v3/email.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/website/src/libs/api-v3/email.js b/website/src/libs/api-v3/email.js index c201410d60..65cbcce02a 100644 --- a/website/src/libs/api-v3/email.js +++ b/website/src/libs/api-v3/email.js @@ -25,13 +25,9 @@ let smtpTransporter = createTransport({ // Send email directly from the server using the smtpTransporter, // used only to send password reset emails because users unsubscribed on Mandrill wouldn't get them export function send (mailData) { - if (IS_PROD) { - return smtpTransporter - .sendMail(mailData) - .catch((error) => logger.error(error)); - } else { - return { send: () => {} }; // mock - } + return smtpTransporter + .sendMail(mailData) + .catch((error) => logger.error(error)); } export function getUserInfo (user, fields = []) { From f73141f1f68e544d876085f7bd3a8fc7337639c9 Mon Sep 17 00:00:00 2001 From: Victor Piousbox Date: Sun, 20 Mar 2016 19:00:23 +0000 Subject: [PATCH 7/7] strengthen the test of password-reset just a little --- .../v3/integration/user/auth/POST-user_reset_password.test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js index 34612106d6..6116f67a35 100644 --- a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js +++ b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js @@ -12,10 +12,13 @@ describe('POST /user/reset-password', async () => { }); it('resets password', async () => { + let previousPassword = user.auth.local.hashed_password; let response = await user.post(endpoint, { email: user.auth.local.email, }); expect(response).to.eql({ message: t('passwordReset') }); + await user.sync(); + expect(user.auth.local.hashed_password).to.not.eql(previousPassword); }); it('same message on error as on success', async () => {