Skip to content

Commit

Permalink
🎨 remove token logic from user model (#7622)
Browse files Browse the repository at this point in the history
* 🔥  remove User model functions

- validateToken
- generateToken
- resetPassword
- all this logic will re-appear in a different way

Token logic:
- was already extracted as separate PR, see #7554
- we will use this logic in the controller, you will see in the next commits

Reset Password:
Was just a wrapper for calling the token logic and change the password.
We can reconsider keeping the function to call: changePassword and activate the status of the user - but i think it's fine to trigger these two actions from the controlling unit.

* 🔥  remove password reset tests from User model

- we already have unit tests for change password and the token logic
- i will re-check at the end if any test case is missing - but for now i will just burn the tests

* ✨  add token logic to controlling unit

generateResetToken endpoint
- the only change here is instead of calling the User model to generate a token, we generate the token via utils
- we fetch the user by email, and generate a hash and return

resetPassword endpoint
- here we have changed a little bit more
- first of all: we have added the validation check if the new passwords match
- a new helper method to extract the token informations
- the brute force security check, which can be handled later from the new bruteforce middleware (see TODO)
- the actual reset function is doing the steps: load me the user, compare the token, change the password and activate the user
- we can think of wrapping these steps into a User model function
- i was not sure about it, because it is actually part of the controlling unit

[ci skip]

* 🎨  tidy up

- jscs
- jshint
- naming functions
- fixes

* ✨  add a test for resetting the password

- there was none
- added a test to reset the password

* 🎨  add more token tests

- ensure quality
- ensure logic we had

* 🔥  remove compare new password check from User Model

- this part of controlling unit

* ✨  compare new passwords for user endpoint

- we deleted the logic in User Model
- we are adding the logic to controlling unit

* 🐛  spam prevention forgotten can crash

- no validation happend before this middleware
- it just assumes that the root key is present
- when we work on our API, we need to ensure that
  1. pre validation happens
  2. we call middlewares
  3. ...

* 🎨  token translation key
  • Loading branch information
kirrg001 authored and ErisDS committed Nov 7, 2016
1 parent a6226e4 commit 4e7779b
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 299 deletions.
158 changes: 119 additions & 39 deletions core/server/api/authentication.js
Expand Up @@ -13,7 +13,8 @@ var _ = require('lodash'),
events = require('../events'),
config = require('../config'),
i18n = require('../i18n'),
authentication;
authentication,
tokenSecurity = {};

/**
* Returns setup status
Expand Down Expand Up @@ -164,14 +165,14 @@ authentication = {

/**
* @description generate a reset token for a given email address
* @param {Object} resetRequest
* @param {Object} object
* @returns {Promise<Object>} 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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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<Object>} 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() {
Expand All @@ -288,13 +366,15 @@ authentication = {
}

tasks = [
assertSetupCompleted(true),
validateRequest,
assertSetupCompleted(true),
extractTokenParts,
protectBruteForce,
doReset,
formatResponse
];

return pipeline(tasks, resetRequest);
return pipeline(tasks, object, options);
},

/**
Expand Down
17 changes: 16 additions & 1 deletion core/server/api/users.js
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions core/server/middleware/api/spam-prevention.js
Expand Up @@ -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,
Expand Down

0 comments on commit 4e7779b

Please sign in to comment.