Reset magic link rate limiting upon successful login (#15345)

refs https://github.com/TryGhost/Team/issues/1771

We don't have access to `req.brute.reset` due to the way the flow
works, we have one endpoint which sends an email with a magic link,
and another route which handles the login. We don't want to apply
brute force protection to both because our rate limiting is designed
for API requests not web page visits (which is how login is handled).

Because of this we require access to the underlying ExpressBrute
instance exposed by the spam-protection module, so that we can
perform the reset.

ghost/core/core/server/services/members/middleware.js

@@ -3,6 +3,7 @@ const logging = require('@tryghost/logging');
 const membersService = require('./service');
 const models = require('../../models');
 const urlUtils = require('../../../shared/url-utils');
+const spamPrevention = require('../../web/shared/middleware/api/spam-prevention');
 const {formattedMemberResponse} = require('./utils');
 
 // @TODO: This piece of middleware actually belongs to the frontend, not to the member app
@@ -155,6 +156,7 @@ const createSessionFromMagicLink = async function (req, res, next) {
 
     try {
         const member = await membersService.ssr.exchangeTokenForSession(req, res);
+        spamPrevention.membersAuth().reset(req.ip, `${member.email}login`);
         const subscriptions = member && member.subscriptions || [];
 
         const action = req.query.action;

ghost/core/core/server/web/shared/middleware/api/spam-prevention.js

@@ -36,6 +36,7 @@ let privateBlogInstance;
 let globalResetInstance;
 let globalBlockInstance;
 let userLoginInstance;
+let membersAuthInstance;
 let userResetInstance;
 let contentApiKeyInstance;
 
@@ -121,6 +122,36 @@ const globalReset = () => {
     return globalResetInstance;
 };
 
+const membersAuth = () => {
+    const ExpressBrute = require('express-brute');
+    const BruteKnex = require('brute-knex');
+    const db = require('../../../../data/db');
+
+    store = store || new BruteKnex({
+        tablename: 'brute',
+        createTable: false,
+        knex: db.knex
+    });
+
+    if (!membersAuthInstance) {
+        membersAuthInstance = new ExpressBrute(store,
+            extend({
+                attachResetToRequest: true,
+                failCallback(req, res, next, nextValidRequestDate) {
+                    return next(new errors.TooManyRequestsError({
+                        message: `Too many sign-in attempts try again in ${moment(nextValidRequestDate).fromNow(true)}`,
+                        context: tpl(messages.tooManySigninAttempts.context),
+                        help: tpl(messages.tooManySigninAttempts.context)
+                    }));
+                },
+                handleStoreError: handleStoreError
+            }, pick(spamUserLogin, spamConfigKeys))
+        );
+    }
+
+    return membersAuthInstance;
+};
+
 // Stops login attempts for a user+IP pair with an increasing time period starting from 10 minutes
 // and rising to a week in a fibonnaci sequence
 // The user+IP count is reset when on successful login
@@ -249,6 +280,7 @@ module.exports = {
     globalBlock: globalBlock,
     globalReset: globalReset,
     userLogin: userLogin,
+    membersAuth: membersAuth,
     userReset: userReset,
     privateBlog: privateBlog,
     contentApiKey: contentApiKey

ghost/core/core/server/web/shared/middleware/brute.js

@@ -86,7 +86,7 @@ module.exports = {
     /**
      */
     membersAuth(req, res, next) {
-        return spamPrevention.userLogin().getMiddleware({
+        return spamPrevention.membersAuth().getMiddleware({
             ignoreIP: false,
             key(_req, _res, _next) {
                 if (_req.body.email) {

ghost/core/test/e2e-api/members/signin.test.js

@@ -1,4 +1,4 @@
-const {agentProvider, mockManager, fixtureManager} = require('../../utils/e2e-framework');
+const {agentProvider, mockManager, fixtureManager, dbUtils, configUtils} = require('../../utils/e2e-framework');
 const models = require('../../../core/server/models');
 const assert = require('assert');
 require('should');
@@ -33,6 +33,7 @@ describe('Members Signin', function () {
     });
 
     beforeEach(function () {
+        mockManager.mockMail();
         mockManager.mockStripe();
     });
 
@@ -119,6 +120,75 @@ describe('Members Signin', function () {
         });
     });
 
+    describe('Rate limiting', function () {
+        it('Will clear rate limits for members auth', async function () {
+            await dbUtils.truncate('brute');
+            // +1 because this is a retry count, so we have one request + the retries, then blocked
+            const userLoginRateLimit = configUtils.config.get('spam').user_login.freeRetries + 1;
+
+            for (let i = 0; i < userLoginRateLimit; i++) {
+                await membersAgent.post('/api/send-magic-link')
+                    .body({
+                        email: 'rate-limiting-test-1@test.com',
+                        emailType: 'signup'
+                    });
+
+                await membersAgent.post('/api/send-magic-link')
+                    .body({
+                        email: 'rate-limiting-test-2@test.com',
+                        emailType: 'signup'
+                    });
+            }
+
+            // Now we've been rate limited
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'rate-limiting-test-1@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Now we've been rate limited
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'rate-limiting-test-2@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Get one of the magic link emails
+            const mail = mockManager.assert.sentEmail({
+                to: 'rate-limiting-test-1@test.com',
+                subject: /Complete your sign up to Ghost!/
+            });
+
+            // Get link from email
+            const [url] = mail.text.match(/https?:\/\/[^\s]+/);
+
+            const magicLink = new URL(url);
+
+            // Login
+            await membersAgent.get(magicLink.pathname + magicLink.search);
+
+            // The first member has been un ratelimited
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'rate-limiting-test-1@test.com',
+                    emailType: 'signup'
+                })
+                .expectEmptyBody()
+                .expectStatus(201);
+
+            // The second is still rate limited
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'rate-limiting-test-2@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+        });
+    });
+
     describe('Member attribution', function () {
         it('Will create a member attribution if magic link contains an attribution source', async function () {
             const email = 'non-existent-member@test.com';

ghost/core/test/unit/server/services/members/middleware.test.js

@@ -107,7 +107,7 @@ describe('Members Service Middleware', function () {
             req.query = {token: 'test', action: 'signup'};
 
             // Fake token handling failure
-            membersService.ssr.exchangeTokenForSession.resolves();
+            membersService.ssr.exchangeTokenForSession.resolves({});
 
             // Fake welcome page for free tier
             models.Product.findOne.resolves({
@@ -153,7 +153,7 @@ describe('Members Service Middleware', function () {
             req.query = {token: 'test', action: 'signin', r: 'https://site.com/blah/my-post/'};
 
             // Fake token handling failure
-            membersService.ssr.exchangeTokenForSession.resolves();
+            membersService.ssr.exchangeTokenForSession.resolves({});
 
             // Call the middleware
             await membersMiddleware.createSessionFromMagicLink(req, res, next);
@@ -169,7 +169,7 @@ describe('Members Service Middleware', function () {
             req.query = {token: 'test', action: 'signin', r: 'https://external.com/whatever/'};
 
             // Fake token handling failure
-            membersService.ssr.exchangeTokenForSession.resolves();
+            membersService.ssr.exchangeTokenForSession.resolves({});
 
             // Call the middleware
             await membersMiddleware.createSessionFromMagicLink(req, res, next);