Merge pull request #3595 from sebgie/issue#3544

Improve spam prevention

config.example.js

@@ -82,8 +82,6 @@ config = {
             host: '127.0.0.1',
             port: '2369'
         },
-        ratePeriod: 1,
-        spamTimeout: 5,
         logging: false
     },
 
@@ -105,8 +103,6 @@ config = {
             host: '127.0.0.1',
             port: '2369'
         },
-        ratePeriod: 1,
-        spamTimeout: 5,
         logging: false
     },
 
@@ -124,8 +120,6 @@ config = {
                 charset  : 'utf8'
             }
         },
-        ratePeriod: 1,
-        spamTimeout: 5,
         server: {
             host: '127.0.0.1',
             port: '2369'

core/server/middleware/index.js

@@ -232,7 +232,7 @@ setupMiddleware = function (server) {
     expressServer = server;
     middleware.cacheServer(expressServer);
     middleware.cacheOauthServer(oauthServer);
-    oauth.init(oauthServer);
+    oauth.init(oauthServer, middleware.resetSpamCounter);
 
     // Make sure 'req.secure' is valid for proxied requests
     // (X-Forwarded-Proto header will be checked, if present)

core/server/middleware/middleware.js

@@ -14,7 +14,8 @@ var _           = require('lodash'),
 
     expressServer,
     oauthServer,
-    loginSecurity = [];
+    loginSecurity = [],
+    forgottenSecurity = [];
 
 function isBlackListedFileType(file) {
     var blackListedFileTypes = ['.hbs', '.md', '.json'],
@@ -132,44 +133,90 @@ var middleware = {
             express['static'](path.join(config.paths.themePath, activeTheme.value), {maxAge: utils.ONE_YEAR_MS})(req, res, next);
         });
     },
-
     // ### Spam prevention Middleware
-    // limit signin requests to one every two seconds
+    // limit signin requests to ten failed requests per IP per hour
-    spamPrevention: function (req, res, next) {
+    spamSigninPrevention: function (req, res, next) {
         var currentTime = process.hrtime()[0],
             remoteAddress = req.connection.remoteAddress,
-            deniedSpam = '',
             deniedRateLimit = '',
             ipCount = '',
-            spamTimeout = config.spamTimeout || 2,
+            rateSigninPeriod = config.rateSigninPeriod || 3600,
-            ratePeriod = config.ratePeriod || 3600,
+            rateSigninAttempts = config.rateSigninAttempts || 10;
-            rateAttempts = config.rateAttempts || 5;
 
-        // filter for IPs that tried to login in the last 2 sec
+        if (req.body.username) {
-        loginSecurity = _.filter(loginSecurity, function (logTime) {
+            loginSecurity.push({ip: remoteAddress, time: currentTime, email: req.body.username});
-            return (logTime.time + ratePeriod > currentTime);
-        });
-        
-
-        // check if IP tried to login in the last 2 sec
-        deniedSpam = _.find(loginSecurity, function (logTime) {
-            return (logTime.time + spamTimeout > currentTime && logTime.ip === remoteAddress);
-        });
-        
-        // check if IP tried to login more than 'rateAttempts' time in the last 'ratePeriod' seconds
-        ipCount = _.chain(loginSecurity).countBy('ip').value();
-        deniedRateLimit = (ipCount[remoteAddress] > rateAttempts);
-
-        if ((!deniedSpam && !deniedRateLimit) || expressServer.get('disableLoginLimiter') === true) {
-            loginSecurity.push({ip: remoteAddress, time: currentTime});
-            next();
         } else {
-            if (deniedRateLimit) {
+            return next(new errors.BadRequestError('No username.'));
-                return next(new errors.UnauthorizedError('Only ' + rateAttempts + ' tries per IP address every ' + ratePeriod + ' seconds.'));
-            } else {
-                return next(new errors.UnauthorizedError('Slow down, there are way too many login attempts!'));
-            }
         }
+
+        // filter entries that are older than rateSigninPeriod
+        loginSecurity = _.filter(loginSecurity, function (logTime) {
+            return (logTime.time + rateSigninPeriod > currentTime);
+        });
+
+        // check number of tries per IP address
+        ipCount = _.chain(loginSecurity).countBy('ip').value();
+        deniedRateLimit = (ipCount[remoteAddress] > rateSigninAttempts);
+
+        if (deniedRateLimit) {
+            return next(new errors.UnauthorizedError('Only ' + rateSigninAttempts + ' tries per IP address every ' + rateSigninPeriod + ' seconds.'));
+        }
+        next();
+    },
+
+    // ### Spam prevention Middleware
+    // 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
+    spamForgottenPrevention: function (req, res, next) {
+        var currentTime = process.hrtime()[0],
+            remoteAddress = req.connection.remoteAddress,
+            rateForgottenPeriod = config.rateForgottenPeriod || 3600,
+            rateForgottenAttempts = config.rateForgottenAttempts || 5,
+            email = req.body.passwordreset[0].email,
+            ipCount = '',
+            deniedRateLimit = '',
+            deniedEmailRateLimit = '',
+            index = _.findIndex(forgottenSecurity, function (logTime) {
+                return (logTime.ip === remoteAddress && logTime.email === email);
+            });
+
+        if (email) {
+            if (index !== -1) {
+                forgottenSecurity[index].count = forgottenSecurity[index].count + 1;
+            } else {
+                forgottenSecurity.push({ip: remoteAddress, time: currentTime, email: email, count: 0});
+            }
+        } else {
+            return next(new errors.BadRequestError('No email.'));
+        }
+
+        // filter entries that are older than rateForgottenPeriod
+        forgottenSecurity = _.filter(forgottenSecurity, function (logTime) {
+            return (logTime.time + rateForgottenPeriod > currentTime);
+        });
+
+        // check number of tries with different email addresses per IP
+        ipCount = _.chain(forgottenSecurity).countBy('ip').value();
+        deniedRateLimit = (ipCount[remoteAddress] > rateForgottenAttempts);
+
+        if (index !== -1) {
+            deniedEmailRateLimit = (forgottenSecurity[index].count > rateForgottenAttempts);
+        }
+        
+        if (deniedEmailRateLimit) {
+            return next(new errors.UnauthorizedError('Only ' + rateForgottenAttempts + ' forgotten password attempts per email every ' + rateForgottenPeriod + ' seconds.'));
+        }
+
+        if (deniedRateLimit) {
+            return next(new errors.UnauthorizedError('Only ' + rateForgottenAttempts + ' tries per IP address every ' + rateForgottenPeriod + ' seconds.'));
+        }
+
+        next();
+    },
+    resetSpamCounter: function (email) {
+        loginSecurity = _.filter(loginSecurity, function (logTime) {
+            return (logTime.email !== email);
+        });
     },
 
     // work around to handle missing client_secret

core/server/middleware/oauth.js

@@ -7,7 +7,7 @@ var oauth2orize = require('oauth2orize'),
 
 oauth = {
 
-    init: function (oauthServer) {
+    init: function (oauthServer, resetSpamCounter) {
 
         // remove all expired accesstokens on startup
         models.Accesstoken.destroyAllExpired();
@@ -39,6 +39,7 @@ oauth = {
                     return models.Accesstoken.add({token: accessToken, user_id: user.id, client_id: client.id, expires: accessExpires}).then(function () {
                         return models.Refreshtoken.add({token: refreshToken, user_id: user.id, client_id: client.id, expires: refreshExpires});
                     }).then(function () {
+                        resetSpamCounter(username);
                         return done(null, accessToken, refreshToken, {expires_in: utils.ONE_HOUR_S});
                     }).catch(function () {
                         return done(null, false);

core/server/routes/api.js

@@ -72,7 +72,7 @@ apiRoutes = function (middleware) {
 
     // ## Authentication
     router.post('/authentication/passwordreset',
-        middleware.spamPrevention,
+        middleware.spamForgottenPrevention,
         api.http(api.authentication.generateResetToken)
     );
     router.put('/authentication/passwordreset', api.http(api.authentication.resetPassword));
@@ -80,7 +80,7 @@ apiRoutes = function (middleware) {
     router.post('/authentication/setup', api.http(api.authentication.setup));
     router.get('/authentication/setup', api.http(api.authentication.isSetup));
     router.post('/authentication/token',
-        middleware.spamPrevention,
+        middleware.spamSigninPrevention,
         middleware.addClientSecret,
         middleware.authenticateClient,
         middleware.generateAccessToken

core/test/functional/client/signin_test.js

@@ -30,43 +30,6 @@ CasperTest.begin('Redirects login to signin', 2, function suite(test) {
     });
 }, true);
 
-// CasperTest.begin('Can\'t spam it', 4, function suite(test) {
-//     CasperTest.Routines.signout.run(test);
-
-//     casper.thenOpenAndWaitForPageLoad('signin', function testTitle() {
-//         test.assertTitle('Ghost Admin', 'Ghost admin has no title');
-//         test.assertUrlMatch(/ghost\/signin\/$/, 'Landed on the correct URL');
-//     });
-
-//     casper.waitForOpaque('.login-box',
-//         function then() {
-//             this.fillAndSave('#login', falseUser);
-//         },
-//         function onTimeout() {
-//             test.fail('Sign in form didn\'t fade in.');
-//         });
-
-
-//     casper.captureScreenshot('login_spam_test.png');
-
-//     casper.waitForText('attempts remaining!', function then() {
-//         this.fillAndSave('#login', falseUser);
-//     });
-
-//     casper.captureScreenshot('login_spam_test2.png');
-
-//     casper.waitForText('Slow down, there are way too many login attempts!', function onSuccess() {
-//         test.assert(true, 'Spamming the login did result in an error notification');
-//         test.assertSelectorDoesntHaveText('.notification-error', '[object Object]');
-//     }, function onTimeout() {
-//         test.assert(false, 'Spamming the login did not result in an error notification');
-//     });
-
-//     // This test causes the spam notification
-//     // add a wait to ensure future tests don't get tripped up by this.
-//     casper.wait(2000);
-// }, true);
-
 CasperTest.begin('Login limit is in place', 4, function suite(test) {
     CasperTest.Routines.signout.run(test);
 
@@ -93,9 +56,6 @@ CasperTest.begin('Login limit is in place', 4, function suite(test) {
     }, function onTimeout() {
         test.assert(false, 'We did not trip the login limit.');
     });
-    // This test used login, add a wait to
-    // ensure future tests don't get tripped up by this.
-    casper.wait(2000);
 }, true);
 
 CasperTest.begin('Can login to Ghost', 5, function suite(test) {

core/test/functional/routes/api/db_test.js

@@ -16,7 +16,6 @@ describe('DB API', function () {
 
     before(function (done) {
         var app = express();
-            app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves

core/test/functional/routes/api/notifications_test.js

@@ -15,7 +15,6 @@ describe('Notifications API', function () {
 
     before(function (done) {
         var app = express();
-            app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves

core/test/functional/routes/api/posts_test.js

@@ -17,7 +17,6 @@ describe('Post API', function () {
 
     before(function (done) {
         var app = express();
-            app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves

core/test/functional/routes/api/settings_test.js

@@ -15,7 +15,6 @@ describe('Settings API', function () {
 
     before(function (done) {
         var app = express();
-        app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves

core/test/functional/routes/api/slugs_test.js

@@ -15,7 +15,6 @@ describe('Slug API', function () {
 
     before(function (done) {
         var app = express();
-        app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves

core/test/functional/routes/api/tags_test.js

@@ -15,7 +15,6 @@ describe('Tag API', function () {
 
     before(function (done) {
         var app = express();
-        app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves

core/test/functional/routes/api/users_test.js

@@ -15,7 +15,6 @@ describe('User API', function () {
 
     before(function (done) {
         var app = express();
-        app.set('disableLoginLimiter', true);
 
         // starting ghost automatically populates the db
         // TODO: prevent db init, and manage bringing up the DB with fixtures ourselves