From d30e7b9251e4308eacc45d0f4c8c8aaf25543800 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Tue, 14 Feb 2017 18:08:31 +0100 Subject: [PATCH] Don't send plaintext reset passwords via email (#8457) * start work to avoid sending reset password in plaintext via email * start checking parameters * fix new password reset email * render error if password reset code is missing or invalid * implement POST route, conversion to bcrypt and messages * add auth.local.passwordResetCode field * add failing tests, move reset code validation func to lib, fixes, remove old tests * fix unit tests * fix page rendering and add integration tests * fix password reset page * add integration test * fix string * fix tests url --- .../GET-auth_reset-password-set-new-one.js | 140 +++++++++ .../POST-auth_reset-password-set-new-one.js | 267 ++++++++++++++++++ .../auth/POST-user_reset_password.test.js | 48 ++-- test/api/v3/unit/libs/password.test.js | 97 +++++++ website/common/locales/en/front.json | 7 +- website/common/locales/en/generic.json | 2 +- website/common/locales/en/settings.json | 2 + website/server/controllers/api-v3/auth.js | 21 +- website/server/controllers/top-level/auth.js | 82 ++++++ website/server/libs/password.js | 37 +++ website/server/models/user/schema.js | 4 +- .../auth/reset-password-set-new-one.jade | 27 ++ 12 files changed, 690 insertions(+), 44 deletions(-) create mode 100644 test/api/v3/integration/user/auth/GET-auth_reset-password-set-new-one.js create mode 100644 test/api/v3/integration/user/auth/POST-auth_reset-password-set-new-one.js create mode 100644 website/views/auth/reset-password-set-new-one.jade diff --git a/test/api/v3/integration/user/auth/GET-auth_reset-password-set-new-one.js b/test/api/v3/integration/user/auth/GET-auth_reset-password-set-new-one.js new file mode 100644 index 0000000000..29d3807e87 --- /dev/null +++ b/test/api/v3/integration/user/auth/GET-auth_reset-password-set-new-one.js @@ -0,0 +1,140 @@ +import { + encrypt, +} from '../../../../../../website/server/libs/encryption'; +import moment from 'moment'; +import { + generateUser, +} from '../../../../../helpers/api-integration/v3'; +import superagent from 'superagent'; +import nconf from 'nconf'; + +const API_TEST_SERVER_PORT = nconf.get('PORT'); + +describe('GET /user/auth/local/reset-password-set-new-one', () => { + let endpoint = `http://localhost:${API_TEST_SERVER_PORT}/static/user/auth/local/reset-password-set-new-one`; + + // Tests to validate the validatePasswordResetCodeAndFindUser function + + it('renders an error page if the code is missing', async () => { + try { + await superagent.get(endpoint); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code is invalid json', async () => { + try { + await superagent.get(`${endpoint}?code=invalid`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code cannot be decrypted', async () => { + let user = await generateUser(); + + try { + let code = JSON.stringify({ // not encrypted + userId: user._id, + expiresAt: new Date(), + }); + await superagent.get(`${endpoint}?code=${code}`); + + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code is expired', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().subtract({minutes: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + try { + await superagent.get(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the user does not exist', async () => { + let code = encrypt(JSON.stringify({ + userId: Date.now().toString(), + expiresAt: moment().add({days: 1}), + })); + + try { + await superagent.get(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the user has no local auth', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + auth: 'not an object with valid fields', + }); + + try { + await superagent.get(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code doesn\'t match the one saved at user.auth.passwordResetCode', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': 'invalid', + }); + + try { + await superagent.get(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + // + + it('returns the password reset page if the password reset code is valid', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + let res = await superagent.get(`${endpoint}?code=${code}`); + expect(res.status).to.equal(200); + }); +}); + diff --git a/test/api/v3/integration/user/auth/POST-auth_reset-password-set-new-one.js b/test/api/v3/integration/user/auth/POST-auth_reset-password-set-new-one.js new file mode 100644 index 0000000000..a963ced5d7 --- /dev/null +++ b/test/api/v3/integration/user/auth/POST-auth_reset-password-set-new-one.js @@ -0,0 +1,267 @@ +import { + encrypt, +} from '../../../../../../website/server/libs/encryption'; +import { + compare, + bcryptCompare, + sha1MakeSalt, + sha1Encrypt as sha1EncryptPassword, +} from '../../../../../../website/server/libs/password'; +import moment from 'moment'; +import { + generateUser, +} from '../../../../../helpers/api-integration/v3'; +import superagent from 'superagent'; +import nconf from 'nconf'; + +const API_TEST_SERVER_PORT = nconf.get('PORT'); + +describe('POST /user/auth/local/reset-password-set-new-one', () => { + let endpoint = `http://localhost:${API_TEST_SERVER_PORT}/static/user/auth/local/reset-password-set-new-one`; + + // Tests to validate the validatePasswordResetCodeAndFindUser function + + it('renders an error page if the code is missing', async () => { + try { + await superagent.post(endpoint); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code is invalid json', async () => { + try { + await superagent.post(`${endpoint}?code=invalid`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code cannot be decrypted', async () => { + let user = await generateUser(); + + try { + let code = JSON.stringify({ // not encrypted + userId: user._id, + expiresAt: new Date(), + }); + await superagent.post(`${endpoint}?code=${code}`); + + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code is expired', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().subtract({minutes: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + try { + await superagent.post(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the user does not exist', async () => { + let code = encrypt(JSON.stringify({ + userId: Date.now().toString(), + expiresAt: moment().add({days: 1}), + })); + + try { + await superagent.post(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the user has no local auth', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + auth: 'not an object with valid fields', + }); + + try { + await superagent.post(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders an error page if the code doesn\'t match the one saved at user.auth.passwordResetCode', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': 'invalid', + }); + + try { + await superagent.post(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + // + + it('renders the error page if the new password is missing', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + try { + await superagent.post(`${endpoint}?code=${code}`); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders the error page if the password confirmation is missing', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + try { + await superagent + .post(`${endpoint}?code=${code}`) + .send({newPassword: 'my new password'}); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders the error page if the password confirmation does not match', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + try { + await superagent + .post(`${endpoint}?code=${code}`) + .send({ + newPassword: 'my new password', + confirmPassword: 'not matching', + }); + throw new Error('Request should fail.'); + } catch (err) { + expect(err.status).to.equal(401); + } + }); + + it('renders the success page and save the user', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + let res = await superagent + .post(`${endpoint}?code=${code}`) + .send({ + newPassword: 'my new password', + confirmPassword: 'my new password', + }); + + expect(res.status).to.equal(200); + + await user.sync(); + expect(user.auth.local.passwordResetCode).to.equal(undefined); + expect(user.auth.local.passwordHashMethod).to.equal('bcrypt'); + expect(user.auth.local.salt).to.be.undefined; + let isPassValid = await compare(user, 'my new password'); + expect(isPassValid).to.equal(true); + }); + + it('renders the success page and convert the password from sha1 to bcrypt', async () => { + let user = await generateUser(); + + let textPassword = 'mySecretPassword'; + let salt = sha1MakeSalt(); + let sha1HashedPassword = sha1EncryptPassword(textPassword, salt); + + await user.update({ + 'auth.local.hashed_password': sha1HashedPassword, + 'auth.local.passwordHashMethod': 'sha1', + 'auth.local.salt': salt, + }); + + await user.sync(); + expect(user.auth.local.passwordHashMethod).to.equal('sha1'); + expect(user.auth.local.salt).to.equal(salt); + expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + let res = await superagent + .post(`${endpoint}?code=${code}`) + .send({ + newPassword: 'my new password', + confirmPassword: 'my new password', + }); + + expect(res.status).to.equal(200); + + await user.sync(); + expect(user.auth.local.passwordResetCode).to.equal(undefined); + expect(user.auth.local.passwordHashMethod).to.equal('bcrypt'); + expect(user.auth.local.salt).to.be.undefined; + expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword); + + let isValidPassword = await bcryptCompare('my new password', user.auth.local.hashed_password); + expect(isValidPassword).to.equal(true); + }); +}); 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 bc10c9a06d..ab86bdcb90 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 @@ -2,10 +2,10 @@ import { generateUser, translate as t, } from '../../../../../helpers/api-integration/v3'; +import moment from 'moment'; import { - sha1MakeSalt, - sha1Encrypt as sha1EncryptPassword, -} from '../../../../../../website/server/libs/password'; + decrypt, +} from '../../../../../../website/server/libs/encryption'; describe('POST /user/reset-password', async () => { let endpoint = '/user/reset-password'; @@ -25,33 +25,6 @@ describe('POST /user/reset-password', async () => { expect(user.auth.local.hashed_password).to.not.eql(previousPassword); }); - it('converts user with SHA1 encrypted password to bcrypt encryption', async () => { - let textPassword = 'mySecretPassword'; - let salt = sha1MakeSalt(); - let sha1HashedPassword = sha1EncryptPassword(textPassword, salt); - - await user.update({ - 'auth.local.hashed_password': sha1HashedPassword, - 'auth.local.passwordHashMethod': 'sha1', - 'auth.local.salt': salt, - }); - - await user.sync(); - expect(user.auth.local.passwordHashMethod).to.equal('sha1'); - expect(user.auth.local.salt).to.equal(salt); - expect(user.auth.local.hashed_password).to.equal(sha1HashedPassword); - - // update email - await user.post(endpoint, { - email: user.auth.local.email, - }); - - await user.sync(); - expect(user.auth.local.passwordHashMethod).to.equal('bcrypt'); - expect(user.auth.local.salt).to.be.undefined; - expect(user.auth.local.hashed_password).not.to.equal(sha1HashedPassword); - }); - it('same message on error as on success', async () => { let response = await user.post(endpoint, { email: 'nonExistent@email.com', @@ -66,4 +39,19 @@ describe('POST /user/reset-password', async () => { message: t('invalidReqParams'), }); }); + + it('sets a new password reset code on user.auth.local that expires in 1 day', async () => { + expect(user.auth.local.passwordResetCode).to.be.undefined; + + await user.post(endpoint, { + email: user.auth.local.email, + }); + + await user.sync(); + + expect(user.auth.local.passwordResetCode).to.be.a.string; + let decryptedCode = JSON.parse(decrypt(user.auth.local.passwordResetCode)); + expect(decryptedCode.userId).to.equal(user._id); + expect(moment(decryptedCode.expiresAt).isAfter(moment().add({hours: 23}))).to.equal(true); + }); }); diff --git a/test/api/v3/unit/libs/password.test.js b/test/api/v3/unit/libs/password.test.js index 6d79f805ed..aa7316ba68 100644 --- a/test/api/v3/unit/libs/password.test.js +++ b/test/api/v3/unit/libs/password.test.js @@ -1,5 +1,12 @@ /* eslint-disable camelcase */ +import { + encrypt, +} from '../../../../../website/server/libs/encryption'; +import moment from 'moment'; +import { + generateUser, +} from '../../../../helpers/api-integration/v3'; import { sha1Encrypt as sha1EncryptPassword, sha1MakeSalt, @@ -7,6 +14,7 @@ import { bcryptCompare, compare, convertToBcrypt, + validatePasswordResetCodeAndFindUser, } from '../../../../../website/server/libs/password'; describe('Password Utilities', () => { @@ -172,6 +180,95 @@ describe('Password Utilities', () => { }); }); + describe('validatePasswordResetCodeAndFindUser', () => { + it('returns false if the code is missing', async () => { + let res = await validatePasswordResetCodeAndFindUser(); + expect(res).to.equal(false); + }); + + it('returns false if the code is invalid json', async () => { + let res = await validatePasswordResetCodeAndFindUser('invalid json'); + expect(res).to.equal(false); + }); + + it('returns false if the code cannot be decrypted', async () => { + let user = await generateUser(); + let res = await validatePasswordResetCodeAndFindUser(JSON.stringify({ // not encrypted + userId: user._id, + expiresAt: new Date(), + })); + expect(res).to.equal(false); + }); + + it('returns false if the code is expired', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().subtract({minutes: 1}), + })); + + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + let res = await validatePasswordResetCodeAndFindUser(code); + expect(res).to.equal(false); + }); + + it('returns false if the user does not exist', async () => { + let res = await validatePasswordResetCodeAndFindUser(encrypt(JSON.stringify({ + userId: Date.now().toString(), + expiresAt: moment().add({days: 1}), + }))); + expect(res).to.equal(false); + }); + + it('returns false if the user has no local auth', async () => { + let user = await generateUser({ + auth: 'not an object with valid fields', + }); + let res = await validatePasswordResetCodeAndFindUser(encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + }))); + expect(res).to.equal(false); + }); + + it('returns false if the code doesn\'t match the one saved at user.auth.passwordResetCode', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + + await user.update({ + 'auth.local.passwordResetCode': 'invalid', + }); + + let res = await validatePasswordResetCodeAndFindUser(code); + expect(res).to.equal(false); + }); + + it('returns the user if the password reset code is valid', async () => { + let user = await generateUser(); + + let code = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({days: 1}), + })); + + await user.update({ + 'auth.local.passwordResetCode': code, + }); + + let res = await validatePasswordResetCodeAndFindUser(code); + expect(res).not.to.equal(false); + expect(res._id).to.equal(user._id); + }); + }); + describe('bcrypt', () => { describe('Hash', () => { it('returns a hashed string', async () => { diff --git a/website/common/locales/en/front.json b/website/common/locales/en/front.json index 217e13c762..3bb4021fef 100644 --- a/website/common/locales/en/front.json +++ b/website/common/locales/en/front.json @@ -252,10 +252,11 @@ "usernameTaken": "Username already taken.", "passwordConfirmationMatch": "Password confirmation doesn't match password.", "invalidLoginCredentials": "Incorrect username and/or email and/or password.", - "passwordReset": "If we have your email on file, a new password has been sent to your email.", + "passwordResetPage": "Reset Password", + "passwordReset": "If we have your email on file, instructions for setting a new password have 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.", + "passwordResetEmailText": "If you requested a password reset for <%= username %> on Habitica, head to <%= passwordResetLink %> to set a new one. The link will expire after 24 hours. If you haven't requested a password reset, please ignore this email.", + "passwordResetEmailHtml": "If you requested a password reset for <%= username %> on Habitica, \">click here to set a new one. The link will expire after 24 hours.

If you haven't requested a password reset, please ignore this email.", "invalidLoginCredentialsLong": "Uh-oh - your username or password is incorrect.\n- Make sure your username or email is typed correctly.\n- You may have signed up with Facebook, not email. Double-check by trying Facebook login.\n- If you forgot your password, click \"Forgot Password\".", "invalidCredentials": "There is no account that uses those credentials.", "accountSuspended": "Account has been suspended, please contact leslie@habitica.com with your User ID \"<%= userId %>\" for assistance.", diff --git a/website/common/locales/en/generic.json b/website/common/locales/en/generic.json index d94354ace5..555e5a8d12 100644 --- a/website/common/locales/en/generic.json +++ b/website/common/locales/en/generic.json @@ -108,7 +108,7 @@ "buyThis": "Buy this <%= text %> with <%= price %> of your <%= gems %> Gems?", "noReachServer": "Server not currently reachable, try again later", "errorUpCase": "ERROR:", - "newPassSent": "If we have your email on file, a new password has been sent to your email.", + "newPassSent": "If we have your email on file, instructions for setting a new password have been sent to your email.", "serverUnreach": "Server currently unreachable.", "requestError": "Yikes, an error occurred! Please reload the page, your last action may not have been saved correctly.", "seeConsole": "If the error persists, please report it at Help > Report a Bug. If you're familiar with your browser's console, please include any error messages.", diff --git a/website/common/locales/en/settings.json b/website/common/locales/en/settings.json index 3799eb2276..236b884be1 100644 --- a/website/common/locales/en/settings.json +++ b/website/common/locales/en/settings.json @@ -88,6 +88,8 @@ "deleteDo": "Do it, delete my account!", "enterNumber": "Please enter a number between 0 and 24", "fillAll": "Please fill out all fields", + "invalidPasswordResetCode": "The supplied password reset code is invalid or has expired.", + "passwordChangeSuccess": "Your password was successfully changed to the one you just chose. You can now use it to access your account.", "passwordSuccess": "Password successfully changed", "usernameSuccess": "Login Name successfully changed", "emailSuccess": "Email successfully changed", diff --git a/website/server/controllers/api-v3/auth.js b/website/server/controllers/api-v3/auth.js index f33cbfbf7e..9bcdedef78 100644 --- a/website/server/controllers/api-v3/auth.js +++ b/website/server/controllers/api-v3/auth.js @@ -16,11 +16,13 @@ import { model as User } from '../../models/user'; import { model as Group } from '../../models/group'; import { model as EmailUnsubscription } from '../../models/emailUnsubscription'; import { sendTxn as sendTxnEmail } from '../../libs/email'; -import { decrypt } from '../../libs/encryption'; +import { decrypt, encrypt } from '../../libs/encryption'; import { send as sendEmail } from '../../libs/email'; import pusher from '../../libs/pusher'; import common from '../../../common'; +const BASE_URL = nconf.get('BASE_URL'); + let api = {}; // When the user signed up after having been invited to a group, invite them automatically to the group @@ -549,11 +551,14 @@ api.resetPassword = { let user = await User.findOne({ 'auth.local.email': email }).exec(); if (user) { - // use a salt as the new password too (they'll change it later) - let newPassword = passwordUtils.sha1MakeSalt(); + // create an encrypted link to be used to reset the password + const passwordResetCode = encrypt(JSON.stringify({ + userId: user._id, + expiresAt: moment().add({ hours: 24 }), + })); + let link = `${BASE_URL}/static/user/auth/local/reset-password-set-new-one?code=${passwordResetCode}`; - // set new password and make sure it's using bcrypt for hashing - await passwordUtils.convertToBcrypt(user, newPassword); // user is saved a few lines below + user.auth.local.passwordResetCode = passwordResetCode; sendEmail({ from: 'Habitica ', @@ -561,13 +566,11 @@ api.resetPassword = { subject: res.t('passwordResetEmailSubject'), text: res.t('passwordResetEmailText', { username: user.auth.local.username, - newPassword, - baseUrl: nconf.get('BASE_URL'), + passwordResetLink: link, }), html: res.t('passwordResetEmailHtml', { username: user.auth.local.username, - newPassword, - baseUrl: nconf.get('BASE_URL'), + passwordResetLink: link, }), }); diff --git a/website/server/controllers/top-level/auth.js b/website/server/controllers/top-level/auth.js index 90724ab1f4..d547c281e8 100644 --- a/website/server/controllers/top-level/auth.js +++ b/website/server/controllers/top-level/auth.js @@ -1,7 +1,89 @@ +import locals from '../../middlewares/locals'; +import { validatePasswordResetCodeAndFindUser, convertToBcrypt} from '../../libs/password'; + let api = {}; // Internal authentication routes +function renderPasswordResetPage (options = {}) { + // res is express' res, error any error and success if the password was successfully changed + let {res, hasError, success = false, message} = options; + + return res.status(hasError ? 401 : 200).render('auth/reset-password-set-new-one.jade', { + env: res.locals.habitrpg, + success, + hasError, + message, // can be error or success message + }); +} + +// Set a new password after having requested a password reset (GET route to input password) +api.resetPasswordSetNewOne = { + method: 'GET', + url: '/static/user/auth/local/reset-password-set-new-one', + middlewares: [locals], + runCron: false, + async handler (req, res) { + let user = await validatePasswordResetCodeAndFindUser(req.query.code); + let isValidCode = Boolean(user); + + return renderPasswordResetPage({ + res, + hasError: !isValidCode, + message: !isValidCode ? res.t('invalidPasswordResetCode') : null, + }); + }, +}; + +// Set a new password after having requested a password reset (POST route to save password) +api.resetPasswordSetNewOneSubmit = { + method: 'POST', + url: '/static/user/auth/local/reset-password-set-new-one', + middlewares: [locals], + runCron: false, + async handler (req, res) { + let user = await validatePasswordResetCodeAndFindUser(req.query.code); + let isValidCode = Boolean(user); + + if (!isValidCode) return renderPasswordResetPage({ + res, + hasError: true, + message: res.t('invalidPasswordResetCode'), + }); + + let newPassword = req.body.newPassword; + let confirmPassword = req.body.confirmPassword; + + if (!newPassword) { + return renderPasswordResetPage({ + res, + hasError: true, + message: res.t('missingNewPassword'), + }); + } + + if (newPassword !== confirmPassword) { + return renderPasswordResetPage({ + res, + hasError: true, + message: res.t('passwordConfirmationMatch'), + }); + } + + // set new password and make sure it's using bcrypt for hashing + await convertToBcrypt(user, String(newPassword)); + user.auth.local.passwordResetCode = undefined; // Reset saved password reset code + await user.save(); + + return renderPasswordResetPage({ + res, + hasError: false, + success: true, + message: res.t('passwordChangeSuccess'), + }); + }, +}; + // Logout the user from the website. api.logout = { method: 'GET', diff --git a/website/server/libs/password.js b/website/server/libs/password.js index 6e9f3ad57e..c81c836e04 100644 --- a/website/server/libs/password.js +++ b/website/server/libs/password.js @@ -1,6 +1,9 @@ // Utilities for working with passwords import crypto from 'crypto'; import bcrypt from 'bcrypt'; +import { decrypt } from './encryption'; +import moment from 'moment'; +import { model as User } from '../models/user'; const BCRYPT_SALT_ROUNDS = 10; @@ -63,3 +66,37 @@ export async function convertToBcrypt (user, plainTextPassword) { user.auth.local.passwordHashMethod = 'bcrypt'; user.auth.local.hashed_password = await bcryptHash(plainTextPassword); // eslint-disable-line camelcase } + +// Returns the user if a valid password reset code is supplied, otherwise false +export async function validatePasswordResetCodeAndFindUser (code) { + let isCodeValid = true; + + let userId; + let user; + let decryptedPasswordResetCode; + + // wrapping the code in a try to be able to handle the error here + try { + decryptedPasswordResetCode = JSON.parse(decrypt(code || 'invalid')); // also catches missing code + userId = decryptedPasswordResetCode.userId; + let expiresAt = decryptedPasswordResetCode.expiresAt; + + if (moment(expiresAt).isBefore(moment())) throw new Error(); + } catch (err) { + isCodeValid = false; + } + + if (isCodeValid) { + user = await User.findById(userId).exec(); + + // check if user is found and if it's an email & password account + if (!user || !user.auth || !user.auth.local || !user.auth.local.email) { + isCodeValid = false; + } else if (code !== user.auth.local.passwordResetCode) { + // Make sure only the last code can be used + isCodeValid = false; + } + } + + return isCodeValid ? user : false; +} diff --git a/website/server/models/user/schema.js b/website/server/models/user/schema.js index 223c6020e9..d5dd67e44e 100644 --- a/website/server/models/user/schema.js +++ b/website/server/models/user/schema.js @@ -59,7 +59,9 @@ let schema = new Schema({ type: String, enum: ['bcrypt', 'sha1'], }, - salt: String, // Salt for SHA1 encrypted passwords, not stored for bcrypt + salt: String, // Salt for SHA1 encrypted passwords, not stored for bcrypt, + // Used to validate password reset codes and make sure only the most recent one can be used + passwordResetCode: String, }, timestamps: { created: {type: Date, default: Date.now}, diff --git a/website/views/auth/reset-password-set-new-one.jade b/website/views/auth/reset-password-set-new-one.jade new file mode 100644 index 0000000000..921c7f6674 --- /dev/null +++ b/website/views/auth/reset-password-set-new-one.jade @@ -0,0 +1,27 @@ +extends ../static/layout + +block vars + - var layoutEnv = env // needed to pass env variable to ./layout + +block title + title Habitica |  + =env.t('passwordResetPage') + +block content + .row(ng-controller='AccordionCtrl') + .col-md-12 + .page-header + h1=env.t('passwordResetPage') + if message + p.lead(class=hasError ? 'text-danger' : 'text-success')=message + hr + if success !== true + // action empty is necessary to prevent Angular from handling the form itself (# causes issues) + form(method='post', action='') + .form-group + label(for='newPass')=env.t('newPass') + input#newPass.form-control(name='newPassword', type='password', placeholder=env.t('newPass')) + .form-group + label(for='confirmPass')=env.t('confirmPass') + input#confirmPass.form-control(name='confirmPassword', type='password', placeholder=env.t('confirmPass')) + button.btn.btn-default(type='submit')=env.t('submit')