From e2bbf7d2068e441426cae59c0f386d7147fba8b6 Mon Sep 17 00:00:00 2001 From: David Wolfe Date: Thu, 17 Nov 2016 13:02:56 +0000 Subject: [PATCH] Fix brute for token exchanges (#7725) closes #7722 - fixes issue where token exhanges are logged with an undefined email address causing lockouts - use more relevant translations for errors --- core/server/auth/oauth.js | 6 +++-- core/server/middleware/api/spam-prevention.js | 22 ++++++++-------- core/server/middleware/brute.js | 17 +++++++++++-- .../routes/api/spam_prevention_spec.js | 4 +-- core/test/unit/auth/oauth_spec.js | 25 +++++++++++++------ 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/core/server/auth/oauth.js b/core/server/auth/oauth.js index 828f81de64..28b4cbf45d 100644 --- a/core/server/auth/oauth.js +++ b/core/server/auth/oauth.js @@ -9,7 +9,7 @@ var oauth2orize = require('oauth2orize'), oauthServer, oauth; -function exchangeRefreshToken(client, refreshToken, scope, done) { +function exchangeRefreshToken(client, refreshToken, scope, body, authInfo, done) { models.Refreshtoken.findOne({token: refreshToken}) .then(function then(model) { if (!model) { @@ -21,6 +21,7 @@ function exchangeRefreshToken(client, refreshToken, scope, done) { refreshExpires = Date.now() + utils.ONE_WEEK_MS; if (token.expires > Date.now()) { + spamPrevention.userLogin.reset(null, authInfo.ip + body.refresh_token + 'login'); models.Accesstoken.add({ token: accessToken, user_id: token.user_id, @@ -70,7 +71,6 @@ function exchangeAuthorizationCode(req, res, next) { message: i18n.t('errors.middleware.auth.accessDenied') })); } - req.query.code = req.body.authorizationCode; passport.authenticate('ghost', {session: false, failWithError: false}, function authenticate(err, user) { @@ -86,6 +86,8 @@ function exchangeAuthorizationCode(req, res, next) { })); } + spamPrevention.userLogin.reset(null, req.authInfo.ip + req.body.authorizationCode + 'login'); + authenticationAPI.createTokens({}, {context: {client_id: req.client.id, user: user.id}}) .then(function then(response) { res.json({ diff --git a/core/server/middleware/api/spam-prevention.js b/core/server/middleware/api/spam-prevention.js index b1165d7970..c9606f9197 100644 --- a/core/server/middleware/api/spam-prevention.js +++ b/core/server/middleware/api/spam-prevention.js @@ -54,7 +54,7 @@ globalBlock = new ExpressBrute(store, message: 'Too many attempts try again in ' + moment(nextValidRequestDate).fromNow(true), context: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.error', {rfa: spamGlobalBlock.freeRetries + 1 || 5, rfp: spamGlobalBlock.lifetime || 60 * 60}), - help: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.context') + help: i18n.t('errors.middleware.spamprevention.tooManyAttempts') })); }, handleStoreError: handleStoreError @@ -86,10 +86,10 @@ userLogin = new ExpressBrute(store, attachResetToRequest: true, failCallback: function (req, res, next, nextValidRequestDate) { return next(new errors.TooManyRequestsError({ - message: 'Too many attempts try again in ' + moment(nextValidRequestDate).fromNow(true), - context: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.error', - {rfa: spamUserLogin.freeRetries + 1 || 5, rfp: spamUserLogin.lifetime || 60 * 60}), - help: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.context') + message: 'Too many sign-in attempts try again in ' + moment(nextValidRequestDate).fromNow(true), + // TODO add more options to i18n + context: i18n.t('errors.middleware.spamprevention.tooManySigninAttempts.context'), + help: i18n.t('errors.middleware.spamprevention.tooManySigninAttempts.context') })); }, handleStoreError: handleStoreError @@ -104,10 +104,10 @@ userReset = new ExpressBrute(store, attachResetToRequest: true, failCallback: function (req, res, next, nextValidRequestDate) { return next(new errors.TooManyRequestsError({ - message: 'Too many attempts try again in ' + moment(nextValidRequestDate).fromNow(true), - context: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.error', + message: 'Too many password reset attempts try again in ' + moment(nextValidRequestDate).fromNow(true), + context: i18n.t('errors.middleware.spamprevention.forgottenPasswordEmail.error', {rfa: spamUserReset.freeRetries + 1 || 5, rfp: spamUserReset.lifetime || 60 * 60}), - help: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.context') + help: i18n.t('errors.middleware.spamprevention.forgottenPasswordEmail.context') })); }, handleStoreError: handleStoreError @@ -121,13 +121,13 @@ privateBlog = new ExpressBrute(store, attachResetToRequest: false, failCallback: function (req, res, next, nextValidRequestDate) { logging.error(new errors.GhostError({ - message: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.error', + message: i18n.t('errors.middleware.spamprevention.tooManySigninAttempts.error', {rfa: spamPrivateBlog.freeRetries + 1 || 5, rfp: spamPrivateBlog.lifetime || 60 * 60}), - context: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.context') + context: i18n.t('errors.middleware.spamprevention.tooManySigninAttempts.context') })); return next(new errors.GhostError({ - message: 'Too many attempts try again in ' + moment(nextValidRequestDate).fromNow(true) + message: 'Too many private sign-in attempts try again in ' + moment(nextValidRequestDate).fromNow(true) })); }, handleStoreError: handleStoreError diff --git a/core/server/middleware/brute.js b/core/server/middleware/brute.js index 429d22daa3..fa0845ff53 100644 --- a/core/server/middleware/brute.js +++ b/core/server/middleware/brute.js @@ -8,6 +8,7 @@ module.exports = { req.authInfo = req.authInfo || {}; req.authInfo.ip = req.connection.remoteAddress; req.body.connection = req.connection.remoteAddress; + next(req.authInfo.ip); } }), @@ -24,9 +25,21 @@ module.exports = { ignoreIP: true, key: function (req, res, next) { req.authInfo = req.authInfo || {}; - req.authInfo.ip = req.connection.remoteAddress; + req.authInfo.ip = req.connection.remoteAddress || req.ip; // prevent too many attempts for the same username - next(req.authInfo.ip + req.body.username + 'login'); + if (req.body.username) { + return next(req.authInfo.ip + req.body.username + 'login'); + } + + if (req.body.authorizationCode) { + return next(req.authInfo.ip + req.body.authorizationCode + 'login'); + } + + if (req.body.refresh_token) { + return next(req.authInfo.ip + req.body.refresh_token + 'login'); + } + + return next(); } }), userReset: spamPrevention.userReset.getMiddleware({ diff --git a/core/test/functional/routes/api/spam_prevention_spec.js b/core/test/functional/routes/api/spam_prevention_spec.js index 9552e3bb92..6885bdbd09 100644 --- a/core/test/functional/routes/api/spam_prevention_spec.js +++ b/core/test/functional/routes/api/spam_prevention_spec.js @@ -29,7 +29,7 @@ describe('Spam Prevention API', function () { }).then(function (user) { author = user; done(); - }).then(testUtils.clearBruteData) + }) .catch(done); }); @@ -67,7 +67,7 @@ describe('Spam Prevention API', function () { var error = res.body.errors[0]; should.exist(error.errorType); error.errorType.should.eql('TooManyRequestsError'); - error.message.should.eql('Too many attempts try again in 10 minutes'); + error.message.should.eql('Too many sign-in attempts try again in 10 minutes'); done(); }); diff --git a/core/test/unit/auth/oauth_spec.js b/core/test/unit/auth/oauth_spec.js index d52735570e..aec0a7554b 100644 --- a/core/test/unit/auth/oauth_spec.js +++ b/core/test/unit/auth/oauth_spec.js @@ -43,8 +43,8 @@ describe('OAuth', function () { req.client = { slug: 'test' }; - req.authInfo = {}; - req.authInfo.ip = '127.0.0.1'; + req.connection = {remoteAddress: '127.0.0.1'}; + req.authInfo = {ip: '127.0.0.1'}; req.body.grant_type = 'password'; req.body.username = 'username'; @@ -95,6 +95,7 @@ describe('OAuth', function () { slug: 'test' }; + req.authInfo = {ip: '127.0.0.1'}; req.body.grant_type = 'password'; req.body.username = 'username'; req.body.password = 'password'; @@ -116,6 +117,7 @@ describe('OAuth', function () { slug: 'test' }; + req.authInfo = {ip: '127.0.0.1'}; req.body.grant_type = 'password'; req.body.username = 'username'; req.body.password = 'password'; @@ -159,7 +161,8 @@ describe('OAuth', function () { req.client = { slug: 'test' }; - + req.authInfo = {ip: '127.0.0.1'}; + req.connection = {remoteAddress: '127.0.0.1'}; req.body.grant_type = 'refresh_token'; req.body.refresh_token = 'token'; res.setHeader = {}; @@ -204,7 +207,8 @@ describe('OAuth', function () { req.client = { slug: 'test' }; - + req.connection = {remoteAddress: '127.0.0.1'}; + req.authInfo = {ip: '127.0.0.1'}; req.body.grant_type = 'refresh_token'; req.body.refresh_token = 'token'; res.setHeader = {}; @@ -224,7 +228,8 @@ describe('OAuth', function () { req.client = { slug: 'test' }; - + req.connection = {remoteAddress: '127.0.0.1'}; + req.authInfo = {ip: '127.0.0.1'}; req.body.grant_type = 'refresh_token'; req.body.refresh_token = 'token'; res.setHeader = {}; @@ -250,7 +255,8 @@ describe('OAuth', function () { req.client = { slug: 'test' }; - + req.connection = {remoteAddress: '127.0.0.1'}; + req.authInfo = {ip: '127.0.0.1'}; req.body.grant_type = 'refresh_token'; req.body.refresh_token = 'token'; res.setHeader = {}; @@ -296,7 +302,8 @@ describe('OAuth', function () { req.client = { id: 1 }; - + req.authInfo = {ip: '127.0.0.1'}; + req.connection = {remoteAddress: '127.0.0.1'}; req.body.grant_type = 'authorization_code'; req.body.authorizationCode = '1234'; @@ -329,6 +336,8 @@ describe('OAuth', function () { id: 1 }; + req.authInfo = {ip: '127.0.0.1'}; + req.connection = {remoteAddress: '127.0.0.1'}; req.body.grant_type = 'authorization_code'; req.body.authorizationCode = '1234'; @@ -351,7 +360,7 @@ describe('OAuth', function () { req.client = { id: 1 }; - + req.connection = {remoteAddress: '127.0.0.1'}; req.body.grant_type = 'authorization_code'; oAuth.generateAccessToken(req, res, function (err) {