diff --git a/ghost/core/core/server/services/members/middleware.js b/ghost/core/core/server/services/members/middleware.js index 6944564aa2..f898a547f8 100644 --- a/ghost/core/core/server/services/members/middleware.js +++ b/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; diff --git a/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js b/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js index 82f1a32631..80525a2556 100644 --- a/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js +++ b/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 diff --git a/ghost/core/core/server/web/shared/middleware/brute.js b/ghost/core/core/server/web/shared/middleware/brute.js index a1b35146e7..6e83f9f2c0 100644 --- a/ghost/core/core/server/web/shared/middleware/brute.js +++ b/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) { diff --git a/ghost/core/test/e2e-api/members/signin.test.js b/ghost/core/test/e2e-api/members/signin.test.js index 9ffe2f148c..4fec2f5f90 100644 --- a/ghost/core/test/e2e-api/members/signin.test.js +++ b/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'; diff --git a/ghost/core/test/unit/server/services/members/middleware.test.js b/ghost/core/test/unit/server/services/members/middleware.test.js index 55f350fdf7..b5a6860458 100644 --- a/ghost/core/test/unit/server/services/members/middleware.test.js +++ b/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);