diff --git a/core/server/api/app.js b/core/server/api/app.js index 9ba3409843..bdd9a6bc04 100644 --- a/core/server/api/app.js +++ b/core/server/api/app.js @@ -166,9 +166,7 @@ function apiRoutes() { // ## Authentication apiRouter.post('/authentication/passwordreset', - // Prevent more than 5 password resets from an ip in an hour for any email address brute.globalReset, - // Prevent more than 5 password resets in an hour for an email+IP pair brute.userReset, api.http(api.authentication.generateResetToken) ); diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index e15e825024..211afe1165 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -266,7 +266,7 @@ authentication = { * @param {Object} object * @returns {Promise} message */ - resetPassword: function resetPassword(object) { + resetPassword: function resetPassword(object, opts) { var tasks, tokenIsCorrect, dbHash, options = {context: {internal: true}}, tokenParts; function validateRequest() { @@ -341,7 +341,7 @@ authentication = { })); } - spamPrevention.userLogin.reset(null, options.data.connection + tokenParts.email + 'login'); + spamPrevention.userLogin.reset(opts.ip, tokenParts.email + 'login'); return models.User.changePassword({ oldPassword: oldPassword, diff --git a/core/server/api/index.js b/core/server/api/index.js index 3adb1e2580..35807f79d2 100644 --- a/core/server/api/index.js +++ b/core/server/api/index.js @@ -220,7 +220,7 @@ http = function http(apiMethod) { return function apiHandler(req, res, next) { // We define 2 properties for using as arguments in API calls: var object = req.body, - options = _.extend({}, req.file, req.query, req.params, { + options = _.extend({}, req.file, {ip: req.ip}, req.query, req.params, { context: { // @TODO: forward the client and user obj in 1.0 (options.context.user.id) user: ((req.user && req.user.id) || (req.user && models.User.isExternalUser(req.user.id))) ? req.user.id : null, diff --git a/core/server/auth/oauth.js b/core/server/auth/oauth.js index 28b4cbf45d..471f682299 100644 --- a/core/server/auth/oauth.js +++ b/core/server/auth/oauth.js @@ -21,7 +21,8 @@ function exchangeRefreshToken(client, refreshToken, scope, body, authInfo, done) refreshExpires = Date.now() + utils.ONE_WEEK_MS; if (token.expires > Date.now()) { - spamPrevention.userLogin.reset(null, authInfo.ip + body.refresh_token + 'login'); + spamPrevention.userLogin.reset(authInfo.ip, body.refresh_token + 'login'); + models.Accesstoken.add({ token: accessToken, user_id: token.user_id, @@ -42,11 +43,12 @@ function exchangeRefreshToken(client, refreshToken, scope, body, authInfo, done) } // We are required to pass in authInfo in order to reset spam counter for user login function exchangePassword(client, username, password, scope, body, authInfo, done) { - // Validate the client models.Client.findOne({slug: client.slug}) .then(function then(client) { if (!client) { - return done(new errors.NoPermissionError({message: i18n.t('errors.middleware.oauth.invalidClient')}), false); + return done(new errors.NoPermissionError({ + message: i18n.t('errors.middleware.oauth.invalidClient') + }), false); } // Validate the user @@ -55,8 +57,7 @@ function exchangePassword(client, username, password, scope, body, authInfo, don return authenticationAPI.createTokens({}, {context: {client_id: client.id, user: user.id}}); }) .then(function then(response) { - // Reset spam count for username and IP pair - spamPrevention.userLogin.reset(null, authInfo.ip + username + 'login'); + spamPrevention.userLogin.reset(authInfo.ip, username + 'login'); return done(null, response.access_token, response.refresh_token, {expires_in: response.expires_in}); }); }) @@ -86,7 +87,7 @@ function exchangeAuthorizationCode(req, res, next) { })); } - spamPrevention.userLogin.reset(null, req.authInfo.ip + req.body.authorizationCode + 'login'); + spamPrevention.userLogin.reset(req.authInfo.ip, req.body.authorizationCode + 'login'); authenticationAPI.createTokens({}, {context: {client_id: req.client.id, user: user.id}}) .then(function then(response) { @@ -147,6 +148,17 @@ oauth = { // ### Generate access token Middleware // register the oauth2orize middleware for password and refresh token grants generateAccessToken: function generateAccessToken(req, res, next) { + /** + * TODO: + * https://github.com/jaredhanson/oauth2orize/issues/182 + * oauth2orize only offers the option to forward request information via authInfo object + * + * Important: only used for resetting the brute count (access to req.ip) + */ + req.authInfo = { + ip: req.ip + }; + return oauthServer.token()(req, res, next); } }; diff --git a/core/server/middleware/api/spam-prevention.js b/core/server/middleware/api/spam-prevention.js index c9606f9197..d479f5999b 100644 --- a/core/server/middleware/api/spam-prevention.js +++ b/core/server/middleware/api/spam-prevention.js @@ -74,7 +74,7 @@ globalReset = new ExpressBrute(store, })); }, handleStoreError: handleStoreError - }, _.pick(spamGlobalBlock, spamConfigKeys)) + }, _.pick(spamGlobalReset, spamConfigKeys)) ); // Stops login attempts for a user+IP pair with an increasing time period starting from 10 minutes diff --git a/core/server/middleware/brute.js b/core/server/middleware/brute.js index fa0845ff53..bc02380259 100644 --- a/core/server/middleware/brute.js +++ b/core/server/middleware/brute.js @@ -1,63 +1,64 @@ -var spamPrevention = require('./api/spam-prevention'); +var url = require('url'), + spamPrevention = require('./api/spam-prevention'); +/** + * We set ignoreIP to false, because we tell brute-knex to use `req.ip`. + * We can use `req.ip`, because express trust proxy option is enabled. + */ module.exports = { + /** + * block per route per ip + */ globalBlock: spamPrevention.globalBlock.getMiddleware({ - // We want to ignore req.ip and instead use req.connection.remoteAddress - ignoreIP: true, + ignoreIP: false, key: function (req, res, next) { - req.authInfo = req.authInfo || {}; - req.authInfo.ip = req.connection.remoteAddress; - req.body.connection = req.connection.remoteAddress; - - next(req.authInfo.ip); + next(url.parse(req.url).pathname); } }), + /** + * block per route per ip + */ globalReset: spamPrevention.globalReset.getMiddleware({ - ignoreIP: true, + ignoreIP: false, key: function (req, res, next) { - req.authInfo = req.authInfo || {}; - req.authInfo.ip = req.connection.remoteAddress; - // prevent too many attempts for the same email address but keep separate to login brute force prevention - next(req.authInfo.ip); + next(url.parse(req.url).pathname); } }), + /** + * block per user + * username === email! + */ userLogin: spamPrevention.userLogin.getMiddleware({ - ignoreIP: true, + ignoreIP: false, key: function (req, res, next) { - req.authInfo = req.authInfo || {}; - req.authInfo.ip = req.connection.remoteAddress || req.ip; - // prevent too many attempts for the same username if (req.body.username) { - return next(req.authInfo.ip + req.body.username + 'login'); + return next(req.body.username + 'login'); } if (req.body.authorizationCode) { - return next(req.authInfo.ip + req.body.authorizationCode + 'login'); + return next(req.body.authorizationCode + 'login'); } if (req.body.refresh_token) { - return next(req.authInfo.ip + req.body.refresh_token + 'login'); + return next(req.body.refresh_token + 'login'); } return next(); } }), + /** + * block per user + */ userReset: spamPrevention.userReset.getMiddleware({ - ignoreIP: true, + ignoreIP: false, key: function (req, res, next) { - req.authInfo = req.authInfo || {}; - req.authInfo.ip = req.connection.remoteAddress; - // prevent too many attempts for the same email address but keep separate to login brute force prevention - next(req.authInfo.ip + req.body.username + 'reset'); + next(req.body.username + 'reset'); } }), privateBlog: spamPrevention.privateBlog.getMiddleware({ - ignoreIP: true, + ignoreIP: false, key: function (req, res, next) { - req.authInfo = req.authInfo || {}; - req.authInfo.ip = req.connection.remoteAddress; - // prevent too many attempts for the same email address but keep separate to login brute force prevention - next(req.authInfo.ip + 'private'); + next('privateblog'); } }) }; diff --git a/core/test/functional/routes/api/authentication_spec.js b/core/test/functional/routes/api/authentication_spec.js index ce98faeb79..97ecacbdd7 100644 --- a/core/test/functional/routes/api/authentication_spec.js +++ b/core/test/functional/routes/api/authentication_spec.js @@ -2,7 +2,10 @@ var supertest = require('supertest'), should = require('should'), testUtils = require('../../../utils'), user = testUtils.DataGenerator.forModel.users[0], + userForKnex = testUtils.DataGenerator.forKnex.users[0], + models = require('../../../../../core/server/models'), config = require('../../../../../core/server/config'), + utils = require('../../../../../core/server/utils'), ghost = testUtils.startGhost, request; @@ -127,7 +130,9 @@ describe('Authentication API', function () { if (err) { return done(err); } + var refreshToken = res.body.refresh_token; + request.post(testUtils.API.getApiQuery('authentication/token')) .set('Origin', config.get('url')) .send({ @@ -173,4 +178,39 @@ describe('Authentication API', function () { done(); }); }); + + it('reset password', function (done) { + models.Settings + .findOne({key: 'dbHash'}) + .then(function (response) { + var token = utils.tokens.resetToken.generateHash({ + expires: Date.now() + (1000 * 60), + email: user.email, + dbHash: response.attributes.value, + password: userForKnex.password + }); + + request.put(testUtils.API.getApiQuery('authentication/passwordreset')) + .set('Origin', config.get('url')) + .set('Accept', 'application/json') + .send({ + passwordreset: [{ + token: token, + newPassword: 'abcdefgh', + ne2Password: 'abcdefgh' + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err) { + if (err) { + return done(err); + } + + done(); + }); + }) + .catch(done); + }); }); diff --git a/core/test/functional/routes/api/spam_prevention_spec.js b/core/test/functional/routes/api/spam_prevention_spec.js index 6885bdbd09..eefc349ab8 100644 --- a/core/test/functional/routes/api/spam_prevention_spec.js +++ b/core/test/functional/routes/api/spam_prevention_spec.js @@ -1,11 +1,14 @@ var supertest = require('supertest'), should = require('should'), testUtils = require('../../../utils'), + db = require('../../../../../core/server/data/db'), config = require('../../../../../core/server/config'), ghost = testUtils.startGhost, failedLoginAttempt, count, + checkBruteTable, tooManyFailedLoginAttempts, + successLoginAttempt, request; describe('Spam Prevention API', function () { @@ -57,7 +60,8 @@ describe('Spam Prevention API', function () { password: 'wrong-password', client_id: 'ghost-admin', client_secret: 'not_available' - }).expect('Content-Type', /json/) + }) + .expect('Content-Type', /json/) .expect(429) .end(function (err, res) { if (err) { @@ -84,12 +88,14 @@ describe('Spam Prevention API', function () { password: 'wrong-password', client_id: 'ghost-admin', client_secret: 'not_available' - }).expect('Content-Type', /json/) + }) + .expect('Content-Type', /json/) .expect(401) .end(function (err) { if (err) { return done(err); } + if (count < config.get('spam:user_login:freeRetries') + 1) { return failedLoginAttempt(email); } @@ -160,4 +166,66 @@ describe('Spam Prevention API', function () { failedLoginAttempt(owner.email); }); + + it('Ensure reset works: password grant type', function (done) { + count = 0; + + checkBruteTable = function checkBruteTable() { + return db.knex('brute').select(); + }; + + successLoginAttempt = function successLoginAttempt(email) { + request.post(testUtils.API.getApiQuery('authentication/token')) + .set('Origin', config.get('url')) + .send({ + grant_type: 'password', + username: email, + password: 'Sl1m3rson', + client_id: 'ghost-admin', + client_secret: 'not_available' + }).expect('Content-Type', /json/) + .expect(200) + .end(function (err) { + if (err) { + return done(err); + } + + checkBruteTable() + .then(function (rows) { + // if reset works, the key is deleted and only one key remains in the database + // the one key is the key for global block + rows.length.should.eql(1); + done(); + }); + }); + }; + + failedLoginAttempt = function failedLoginAttempt(email) { + count += 1; + + request.post(testUtils.API.getApiQuery('authentication/token')) + .set('Origin', config.get('url')) + .send({ + grant_type: 'password', + username: email, + password: 'wrong-password', + client_id: 'ghost-admin', + client_secret: 'not_available' + }).expect('Content-Type', /json/) + .expect(401) + .end(function (err) { + if (err) { + return done(err); + } + + if (count < config.get('spam:user_login:freeRetries') - 1) { + return failedLoginAttempt(email); + } + + successLoginAttempt(email); + }); + }; + + failedLoginAttempt(owner.email); + }); }); diff --git a/core/test/integration/api/api_authentication_spec.js b/core/test/integration/api/api_authentication_spec.js index e081886733..35b0112d34 100644 --- a/core/test/integration/api/api_authentication_spec.js +++ b/core/test/integration/api/api_authentication_spec.js @@ -348,7 +348,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) {