diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index 8aeadfe36179..93864735a693 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -13,7 +13,8 @@ var _ = require('lodash'), events = require('../events'), config = require('../config'), i18n = require('../i18n'), - authentication; + authentication, + tokenSecurity = {}; /** * Returns setup status @@ -164,14 +165,14 @@ authentication = { /** * @description generate a reset token for a given email address - * @param {Object} resetRequest + * @param {Object} object * @returns {Promise} message */ - generateResetToken: function generateResetToken(resetRequest) { + generateResetToken: function generateResetToken(object) { var tasks; - function validateRequest(resetRequest) { - return utils.checkObject(resetRequest, 'passwordreset').then(function then(data) { + function validateRequest(object) { + return utils.checkObject(object, 'passwordreset').then(function then(data) { var email = data.passwordreset[0].email; if (typeof email !== 'string' || !validator.isEmail(email)) { @@ -185,19 +186,32 @@ authentication = { } function generateToken(email) { - var settingsQuery = {context: {internal: true}, key: 'dbHash'}; + var options = {context: {internal: true}}, + dbHash, token; - return settings.read(settingsQuery).then(function then(response) { - var dbHash = response.settings[0].value, - expiresAt = Date.now() + globalUtils.ONE_DAY_MS; + return settings.read(_.merge({key: 'dbHash'}, options)) + .then(function fetchedSettings(response) { + dbHash = response.settings[0].value; - return models.User.generateResetToken(email, expiresAt, dbHash); - }).then(function then(resetToken) { - return { - email: email, - resetToken: resetToken - }; - }); + return models.User.getByEmail(email, options); + }) + .then(function fetchedUser(user) { + if (!user) { + throw new errors.NotFoundError({message: i18n.t('errors.api.users.userNotFound')}); + } + + token = globalUtils.tokens.resetToken.generateHash({ + expires: Date.now() + globalUtils.ONE_DAY_MS, + email: email, + dbHash: dbHash, + password: user.get('password') + }); + + return { + email: email, + resetToken: token + }; + }); } function sendResetNotification(data) { @@ -244,39 +258,103 @@ authentication = { formatResponse ]; - return pipeline(tasks, resetRequest); + return pipeline(tasks, object); }, /** * ## Reset Password * reset password if a valid token and password (2x) is passed - * @param {Object} resetRequest + * @param {Object} object * @returns {Promise} message */ - resetPassword: function resetPassword(resetRequest) { - var tasks; + resetPassword: function resetPassword(object) { + var tasks, tokenIsCorrect, dbHash, options = {context: {internal: true}}, tokenParts; + + function validateRequest() { + return utils.validate('passwordreset')(object, options) + .then(function (options) { + var data = options.data.passwordreset[0]; + + if (data.newPassword !== data.ne2Password) { + return Promise.reject(new errors.ValidationError({ + message: i18n.t('errors.models.user.newPasswordsDoNotMatch') + })); + } + + return Promise.resolve(options); + }); + } + + function extractTokenParts(options) { + options.data.passwordreset[0].token = globalUtils.decodeBase64URLsafe(options.data.passwordreset[0].token); + + tokenParts = globalUtils.tokens.resetToken.extract({ + token: options.data.passwordreset[0].token + }); + + if (!tokenParts) { + return Promise.reject(new errors.UnauthorizedError({ + message: i18n.t('errors.api.common.invalidTokenStructure') + })); + } - function validateRequest(resetRequest) { - return utils.checkObject(resetRequest, 'passwordreset'); + return Promise.resolve(options); } - function doReset(resetRequest) { - var settingsQuery = {context: {internal: true}, key: 'dbHash'}, - data = resetRequest.passwordreset[0], + // @TODO: use brute force middleware (see https://github.com/TryGhost/Ghost/pull/7579) + function protectBruteForce(options) { + if (tokenSecurity[tokenParts.email + '+' + tokenParts.expires] && + tokenSecurity[tokenParts.email + '+' + tokenParts.expires].count >= 10) { + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.models.user.tokenLocked') + })); + } + + return Promise.resolve(options); + } + + function doReset(options) { + var data = options.data.passwordreset[0], resetToken = data.token, - newPassword = data.newPassword, - ne2Password = data.ne2Password; - - return settings.read(settingsQuery).then(function then(response) { - return models.User.resetPassword({ - token: resetToken, - newPassword: newPassword, - ne2Password: ne2Password, - dbHash: response.settings[0].value + oldPassword = data.oldPassword, + newPassword = data.newPassword; + + return settings.read(_.merge({key: 'dbHash'}, options)) + .then(function fetchedSettings(response) { + dbHash = response.settings[0].value; + + return models.User.getByEmail(tokenParts.email, options); + }) + .then(function fetchedUser(user) { + if (!user) { + throw new errors.NotFoundError({message: i18n.t('errors.api.users.userNotFound')}); + } + + tokenIsCorrect = globalUtils.tokens.resetToken.compare({ + token: resetToken, + dbHash: dbHash, + password: user.get('password') + }); + + if (!tokenIsCorrect) { + return Promise.reject(new errors.BadRequestError({ + message: i18n.t('errors.api.common.invalidTokenStructure') + })); + } + + return models.User.changePassword({ + oldPassword: oldPassword, + newPassword: newPassword, + user_id: user.id + }, options); + }) + .then(function changedPassword(updatedUser) { + updatedUser.set('status', 'active'); + return updatedUser.save(options); + }) + .catch(function (err) { + throw new errors.UnauthorizedError({err: err}); }); - }).catch(function (err) { - throw new errors.UnauthorizedError({err: err}); - }); } function formatResponse() { @@ -288,13 +366,15 @@ authentication = { } tasks = [ - assertSetupCompleted(true), validateRequest, + assertSetupCompleted(true), + extractTokenParts, + protectBruteForce, doReset, formatResponse ]; - return pipeline(tasks, resetRequest); + return pipeline(tasks, object, options); }, /** diff --git a/core/server/api/users.js b/core/server/api/users.js index 0d09d14f6c80..1afd6e8a7795 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -269,6 +269,21 @@ users = { changePassword: function changePassword(object, options) { var tasks; + function validateRequest() { + return utils.validate('password')(object, options) + .then(function (options) { + var data = options.data.password[0]; + + if (data.newPassword !== data.ne2Password) { + return Promise.reject(new errors.ValidationError({ + message: i18n.t('errors.models.user.newPasswordsDoNotMatch') + })); + } + + return Promise.resolve(options); + }); + } + /** * ### Handle Permissions * We need to be an authorised user to perform this action @@ -301,7 +316,7 @@ users = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ - utils.validate('password'), + validateRequest, handlePermissions, utils.convertOptions(allowedIncludes), doQuery diff --git a/core/server/middleware/api/spam-prevention.js b/core/server/middleware/api/spam-prevention.js index 4afa623179fa..fec3f554f748 100644 --- a/core/server/middleware/api/spam-prevention.js +++ b/core/server/middleware/api/spam-prevention.js @@ -54,7 +54,14 @@ spamPrevention = { // limit forgotten password requests to five requests per IP per hour for different email addresses // limit forgotten password requests to five requests per email address + // @TODO: add validation check to validation middleware forgotten: function forgotten(req, res, next) { + if (!req.body.passwordreset) { + return next(new errors.BadRequestError({ + message: i18n.t('errors.api.utils.noRootKeyProvided', {docName: 'passwordreset'}) + })); + } + var currentTime = process.hrtime()[0], remoteAddress = req.connection.remoteAddress, rateForgottenPeriod = config.rateForgottenPeriod || 3600, diff --git a/core/server/models/user.js b/core/server/models/user.js index 040e67a56d11..fc1c31b0a628 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -1,7 +1,6 @@ var _ = require('lodash'), Promise = require('bluebird'), bcrypt = require('bcryptjs'), - crypto = require('crypto'), validator = require('validator'), ghostBookshelf = require('./base'), errors = require('../errors'), @@ -16,7 +15,6 @@ var _ = require('lodash'), bcryptHash = Promise.promisify(bcrypt.hash), bcryptCompare = Promise.promisify(bcrypt.compare), - tokenSecurity = {}, activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked'], User, Users; @@ -680,17 +678,11 @@ User = ghostBookshelf.Model.extend({ changePassword: function changePassword(object, options) { var self = this, newPassword = object.newPassword, - ne2Password = object.ne2Password, userId = parseInt(object.user_id), oldPassword = object.oldPassword, isLoggedInUser = userId === options.context.user, user; - // If the two passwords do not match - if (newPassword !== ne2Password) { - return Promise.reject(new errors.ValidationError({message: i18n.t('errors.models.user.newPasswordsDoNotMatch')})); - } - return self.forge({id: userId}).fetch({require: true}) .then(function then(_user) { user = _user; @@ -707,114 +699,6 @@ User = ghostBookshelf.Model.extend({ }); }, - generateResetToken: function generateResetToken(email, expires, dbHash) { - return this.getByEmail(email).then(function then(foundUser) { - if (!foundUser) { - return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.models.user.noUserWithEnteredEmailAddr')})); - } - - var hash = crypto.createHash('sha256'), - text = ''; - - // Token: - // BASE64(TIMESTAMP + email + HASH(TIMESTAMP + email + oldPasswordHash + dbHash )) - hash.update(String(expires)); - hash.update(email.toLocaleLowerCase()); - hash.update(foundUser.get('password')); - hash.update(String(dbHash)); - - text += [expires, email, hash.digest('base64')].join('|'); - return new Buffer(text).toString('base64'); - }); - }, - - validateToken: function validateToken(token, dbHash) { - /*jslint bitwise:true*/ - // TODO: Is there a chance the use of ascii here will cause problems if oldPassword has weird characters? - var tokenText = new Buffer(token, 'base64').toString('ascii'), - parts, - expires, - email; - - parts = tokenText.split('|'); - - // Check if invalid structure - if (!parts || parts.length !== 3) { - return Promise.reject(new errors.BadRequestError({message: i18n.t('errors.models.user.invalidTokenStructure')})); - } - - expires = parseInt(parts[0], 10); - email = parts[1]; - - if (isNaN(expires)) { - return Promise.reject(new errors.BadRequestError({message: i18n.t('errors.models.user.invalidTokenExpiration')})); - } - - // Check if token is expired to prevent replay attacks - if (expires < Date.now()) { - return Promise.reject(new errors.ValidationError({message: i18n.t('errors.models.user.expiredToken')})); - } - - // to prevent brute force attempts to reset the password the combination of email+expires is only allowed for - // 10 attempts - if (tokenSecurity[email + '+' + expires] && tokenSecurity[email + '+' + expires].count >= 10) { - return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.models.user.tokenLocked')})); - } - - return this.generateResetToken(email, expires, dbHash).then(function then(generatedToken) { - // Check for matching tokens with timing independent comparison - var diff = 0, - i; - - // check if the token length is correct - if (token.length !== generatedToken.length) { - diff = 1; - } - - for (i = token.length - 1; i >= 0; i = i - 1) { - diff |= token.charCodeAt(i) ^ generatedToken.charCodeAt(i); - } - - if (diff === 0) { - return email; - } - - // increase the count for email+expires for each failed attempt - tokenSecurity[email + '+' + expires] = { - count: tokenSecurity[email + '+' + expires] ? tokenSecurity[email + '+' + expires].count + 1 : 1 - }; - return Promise.reject(new errors.BadRequestError({message: i18n.t('errors.models.user.invalidToken')})); - }); - }, - - resetPassword: function resetPassword(options) { - var self = this, - token = options.token, - newPassword = options.newPassword, - ne2Password = options.ne2Password, - dbHash = options.dbHash; - - if (newPassword !== ne2Password) { - return Promise.reject(new errors.ValidationError({ - message: i18n.t('errors.models.user.newPasswordsDoNotMatch') - })); - } - - // Validate the token; returns the email address from token - return self.validateToken(utils.decodeBase64URLsafe(token), dbHash).then(function then(email) { - return self.getByEmail(email); - }).then(function then(foundUser) { - if (!foundUser) { - return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.models.user.userNotFound')})); - } - - return foundUser.save({ - status: 'active', - password: newPassword - }); - }); - }, - transferOwnership: function transferOwnership(object, options) { var ownerRole, contextUser; diff --git a/core/server/translations/en.json b/core/server/translations/en.json index 87f9b5c08422..34d48a10b744 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -245,8 +245,6 @@ "accountLocked": "Your account is locked. Please reset your password to log in again by clicking the \"Forgotten password?\" link!", "newPasswordsDoNotMatch": "Your new passwords do not match", "passwordRequiredForOperation": "Password is required for this operation", - "invalidTokenStructure": "Invalid token structure", - "invalidTokenExpiration": "Invalid token expiration", "expiredToken": "Expired token", "tokenLocked": "Token locked", "invalidToken": "Invalid token", @@ -289,6 +287,9 @@ } }, "api": { + "common": { + "invalidTokenStructure": "Invalid token structure" + }, "authentication": { "setupUnableToRun": "Database missing fixture data. Please reset database and try again.", "setupMustBeCompleted": "Setup must be completed before making this request.", diff --git a/core/server/utils/tokens.js b/core/server/utils/tokens.js index 7c402f52a0b5..5056f57291a3 100644 --- a/core/server/utils/tokens.js +++ b/core/server/utils/tokens.js @@ -11,10 +11,10 @@ exports.resetToken = { password = options.password, text = ''; - hash.update(String(expires).toLocaleLowerCase()); - hash.update(String(email).toLocaleLowerCase()); - hash.update(String(dbHash).toLocaleLowerCase()); - hash.update(String(password).toLocaleLowerCase()); + hash.update(String(expires)); + hash.update(email.toLocaleLowerCase()); + hash.update(password); + hash.update(String(dbHash)); text += [expires, email, hash.digest('base64')].join('|'); return new Buffer(text).toString('base64'); diff --git a/core/test/integration/api/api_authentication_spec.js b/core/test/integration/api/api_authentication_spec.js index f8ee41105691..b6db3477c94b 100644 --- a/core/test/integration/api/api_authentication_spec.js +++ b/core/test/integration/api/api_authentication_spec.js @@ -4,6 +4,7 @@ var testUtils = require('../../utils'), sinon = require('sinon'), Promise = require('bluebird'), uid = require('../../../server/utils').uid, + globalUtils = require('../../../server/utils'), AuthAPI = require('../../../server/api/authentication'), mail = require('../../../server/api/mail'), models = require('../../../server/models'), @@ -14,6 +15,7 @@ var testUtils = require('../../utils'), Refreshtoken, User; +// @TODO: group tests by api call, not by setup completed or not describe('Authentication API', function () { var testInvite = { invitation: [{ @@ -31,6 +33,7 @@ describe('Authentication API', function () { testReset = { passwordreset: [{ token: 'abc', + oldPassword: 'Sl1m3rson', newPassword: 'abcdefgh', ne2Password: 'abcdefgh' }] @@ -347,7 +350,7 @@ describe('Authentication API', function () { }).catch(done); }); - it('should allow a password reset', function (done) { + it('should NOT allow a password reset', function (done) { AuthAPI.resetPassword(testReset).then(function () { done(new Error('password reset did not fail on token validation')); }).catch(function (err) { @@ -361,6 +364,38 @@ describe('Authentication API', function () { }).catch(done); }); + it('should allow a password reset', function (done) { + sandbox.stub(globalUtils.tokens.resetToken, 'generateHash').returns('valid-token'); + sandbox.stub(globalUtils.tokens.resetToken, 'extract').returns({email: 'jbloggs@example.com', expires: Date.now() + (60 * 1000)}); + sandbox.stub(globalUtils.tokens.resetToken, 'compare').returns(true); + + models.User.edit({status: 'locked'}, {id: 1}) + .then(function (user) { + user.get('status').should.eql('locked'); + return AuthAPI.generateResetToken(testGenerateReset); + }) + .then(function () { + return AuthAPI.resetPassword(testReset); + }) + .then(function () { + return models.User.findOne({id: 1}); + }) + .then(function (user) { + user.get('status').should.eql('active'); + + return models.User.isPasswordCorrect({ + plainPassword: testReset.passwordreset[0].newPassword, + hashedPassword: user.get('password') + }); + }) + .then(function () { + done(); + }) + .catch(function (err) { + done(err); + }); + }); + it('should allow an access token to be revoked', function (done) { var id = uid(191); diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index 34ab91ca308b..a7da348e3c20 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -2,11 +2,9 @@ var testUtils = require('../../utils'), should = require('should'), Promise = require('bluebird'), sinon = require('sinon'), - uuid = require('node-uuid'), _ = require('lodash'), // Stuff we are testing - utils = require('../../../server/utils'), errors = require('../../../server/errors'), gravatar = require('../../../server/utils/gravatar'), UserModel = require('../../../server/models/user').User, @@ -591,138 +589,6 @@ describe('User Model', function run() { }); }); - describe('Password Reset', function () { - beforeEach(testUtils.setup('users:roles')); - - it('can generate reset token', function (done) { - // Expires in one minute - var expires = Date.now() + 60000, - dbHash = uuid.v4(); - - UserModel.findAll().then(function (results) { - return UserModel.generateResetToken(results.models[0].attributes.email, expires, dbHash); - }).then(function (token) { - should.exist(token); - - token.length.should.be.above(0); - - done(); - }).catch(done); - }); - - it('can validate a reset token', function (done) { - // Expires in one minute - var expires = Date.now() + 60000, - dbHash = uuid.v4(); - - UserModel.findAll().then(function (results) { - return UserModel.generateResetToken(results.models[1].attributes.email, expires, dbHash); - }).then(function (token) { - return UserModel.validateToken(token, dbHash); - }).then(function () { - done(); - }).catch(done); - }); - - it('can validate an URI encoded reset token', function (done) { - // Expires in one minute - var expires = Date.now() + 60000, - dbHash = uuid.v4(); - - UserModel.findAll().then(function (results) { - return UserModel.generateResetToken(results.models[1].attributes.email, expires, dbHash); - }).then(function (token) { - token = utils.encodeBase64URLsafe(token); - token = encodeURIComponent(token); - token = decodeURIComponent(token); - token = utils.decodeBase64URLsafe(token); - return UserModel.validateToken(token, dbHash); - }).then(function () { - done(); - }).catch(done); - }); - - it('can reset a password with a valid token', function (done) { - // Expires in one minute - var origPassword, - expires = Date.now() + 60000, - dbHash = uuid.v4(); - - UserModel.findAll().then(function (results) { - var firstUser = results.models[0], - origPassword = firstUser.attributes.password; - - should.exist(origPassword); - - return UserModel.generateResetToken(firstUser.attributes.email, expires, dbHash); - }).then(function (token) { - token = utils.encodeBase64URLsafe(token); - return UserModel.resetPassword({token: token, newPassword: 'newpassword', ne2Password: 'newpassword', dbHash: dbHash}); - }).then(function (resetUser) { - var resetPassword = resetUser.get('password'); - - should.exist(resetPassword); - resetPassword.should.not.equal(origPassword); - - done(); - }).catch(done); - }); - - it('doesn\'t allow expired timestamp tokens', function (done) { - var email, - // Expired one minute ago - expires = Date.now() - 60000, - dbHash = uuid.v4(); - - UserModel.findAll().then(function (results) { - // Store email for later - email = results.models[0].attributes.email; - - return UserModel.generateResetToken(email, expires, dbHash); - }).then(function (token) { - return UserModel.validateToken(token, dbHash); - }).then(function () { - throw new Error('Allowed expired token'); - }).catch(function (err) { - should.exist(err); - - err.message.should.equal('Expired token'); - - done(); - }); - }); - - it('doesn\'t allow tampered timestamp tokens', function (done) { - // Expired one minute ago - var expires = Date.now() - 60000, - dbHash = uuid.v4(); - - UserModel.findAll().then(function (results) { - return UserModel.generateResetToken(results.models[0].attributes.email, expires, dbHash); - }).then(function (token) { - var tokenText = new Buffer(token, 'base64').toString('ascii'), - parts = tokenText.split('|'), - fakeExpires, - fakeToken; - - fakeExpires = Date.now() + 60000; - - fakeToken = [String(fakeExpires), parts[1], parts[2]].join('|'); - fakeToken = new Buffer(fakeToken).toString('base64'); - - return UserModel.validateToken(fakeToken, dbHash); - }).then(function () { - throw new Error('allowed invalid token'); - }).catch(function (err) { - should.exist(err); - - err.message.should.equal('Invalid token'); - - done(); - }); - }); - }); - describe('User setup', function () { beforeEach(testUtils.setup('owner')); diff --git a/core/test/unit/utils/tokens_spec.js b/core/test/unit/utils/tokens_spec.js index 2948ceb90ca9..ac7c33935dde 100644 --- a/core/test/unit/utils/tokens_spec.js +++ b/core/test/unit/utils/tokens_spec.js @@ -12,6 +12,7 @@ describe('Utils: tokens', function () { token = utils.tokens.resetToken.generateHash({ email: 'test1@ghost.org', expires: expires, + password: 'password', dbHash: dbHash }); @@ -80,12 +81,34 @@ describe('Utils: tokens', function () { should.not.exist(parts.dbHash); }); + it('extract', function () { + var expires = Date.now() + 60 * 1000, + dbHash = uuid.v4(), token, parts, email = 'test3@ghost.org'; + + token = utils.tokens.resetToken.generateHash({ + email: email, + expires: expires, + password: '$2a$10$t5dY1uRRdjvqfNlXhae3uuc0nuhi.Rd7/K/9JaHHwSkLm6UUa3NsW', + dbHash: dbHash + }); + + parts = utils.tokens.resetToken.extract({ + token: token + }); + + parts.email.should.eql(email); + parts.expires.should.eql(expires); + should.not.exist(parts.password); + should.not.exist(parts.dbHash); + }); + it('can validate an URI encoded reset token', function () { var expires = Date.now() + 60 * 1000, - dbHash = uuid.v4(), token, tokenIsCorrect; + email = 'test1@ghost.org', + dbHash = uuid.v4(), token, tokenIsCorrect, parts; token = utils.tokens.resetToken.generateHash({ - email: 'test1@ghost.org', + email: email, expires: expires, password: '12345678', dbHash: dbHash @@ -96,6 +119,13 @@ describe('Utils: tokens', function () { token = decodeURIComponent(token); token = utils.decodeBase64URLsafe(token); + parts = utils.tokens.resetToken.extract({ + token: token + }); + + parts.email.should.eql(email); + parts.expires.should.eql(expires); + tokenIsCorrect = utils.tokens.resetToken.compare({ token: token, dbHash: dbHash,