🐛 Prevented member creation when logging in (#15526)

fixes https://github.com/TryGhost/Ghost/issues/14508

This change requires the frontend to send an explicit `emailType` when sending a magic link. We default to `subscribe` (`signin` for invite only sites) for now to remain compatible with the existing behaviour.

**Problem:**
When a member tries to login and that member doesn't exist, we created a new member in the past.

- This caused the creation of duplicate accounts when members were guessing the email address they used.
- This caused the creation of new accounts when using an old impersonation token, login link or email change link that was sent before member deletion.

**Fixed:**
- Trying to login with an email address that doesn't exist will throw an error now.
- Added new and separate rate limiting to login (to prevent user enumeration). This rate limiting has a higher default limit of 8. I think it needs a higher default limit (because it is rate limited on every call instead of per email address. And it should be configurable independent from administrator rate limiting. It also needs a lower lifetime value because it is never reset.
- Updated error responses in the `sendMagicLink` endpoint to use the default error encoding middleware.
- The type (`signin`, `signup`, `updateEmail` or `subscribe`) is now stored in the magic link. This is used to prevent signups with a sign in token.

**Notes:**
- Between tests, we truncate the database, but this is not enough for the rate limits to be truly reset. I had to add a method to the spam prevention service to reset all the instances between tests. Not resetting them caused random failures because every login in every test was hitting those spam prevention middlewares and somehow left a trace of that in those instances (even when the brute table is reset). Maybe those instances were doing some in memory caching.

ghost/core/core/server/api/endpoints/member-signin-urls.js

@@ -22,7 +22,7 @@ module.exports = {
                 });
             }
 
-            const magicLink = await membersService.api.getMagicLink(model.get('email'));
+            const magicLink = await membersService.api.getMagicLink(model.get('email'), 'signin');
 
             return {
                 member_id: model.get('id'),

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

@@ -157,6 +157,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`);
+        // Note: don't reset 'member_login', or that would give an easy way around user enumeration by logging in to a manually created account
         const subscriptions = member && member.subscriptions || [];
 
         const action = req.query.action;

ghost/core/core/server/web/members/app.js

@@ -48,7 +48,15 @@ module.exports = function setupMembersApp() {
     membersApp.delete('/api/session', middleware.deleteSession);
 
     // NOTE: this is wrapped in a function to ensure we always go via the getter
-    membersApp.post('/api/send-magic-link', bodyParser.json(), shared.middleware.brute.membersAuth, (req, res, next) => membersService.api.middleware.sendMagicLink(req, res, next));
+    membersApp.post(
+        '/api/send-magic-link', 
+        bodyParser.json(), 
+        // Prevent brute forcing email addresses (user enumeration)
+        shared.middleware.brute.membersAuthEnumeration, 
+        // Prevent brute forcing passwords for the same email address
+        shared.middleware.brute.membersAuth, 
+        (req, res, next) => membersService.api.middleware.sendMagicLink(req, res, next)
+    );
     membersApp.post('/api/create-stripe-checkout-session', (req, res, next) => membersService.api.middleware.createCheckoutSession(req, res, next));
     membersApp.post('/api/create-stripe-update-session', (req, res, next) => membersService.api.middleware.createCheckoutSetupSession(req, res, next));
     membersApp.put('/api/subscriptions/:id', (req, res, next) => membersService.api.middleware.updateSubscription(req, res, next));

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

@@ -5,7 +5,7 @@ const errors = require('@tryghost/errors');
 const config = require('../../../../../shared/config');
 const tpl = require('@tryghost/tpl');
 const logging = require('@tryghost/logging');
-const spam = config.get('spam') || {};
+let spam = config.get('spam') || {};
 
 const messages = {
     forgottenPasswordEmail: {
@@ -22,13 +22,13 @@ const messages = {
     },
     tooManyAttempts: 'Too many attempts.'
 };
-
-const spamPrivateBlock = spam.private_block || {};
-const spamGlobalBlock = spam.global_block || {};
-const spamGlobalReset = spam.global_reset || {};
-const spamUserReset = spam.user_reset || {};
-const spamUserLogin = spam.user_login || {};
-const spamContentApiKey = spam.content_api_key || {};
+let spamPrivateBlock = spam.private_block || {};
+let spamGlobalBlock = spam.global_block || {};
+let spamGlobalReset = spam.global_reset || {};
+let spamUserReset = spam.user_reset || {};
+let spamUserLogin = spam.user_login || {};
+let spamMemberLogin = spam.member_login || {};
+let spamContentApiKey = spam.content_api_key || {};
 
 let store;
 let memoryStore;
@@ -37,6 +37,7 @@ let globalResetInstance;
 let globalBlockInstance;
 let userLoginInstance;
 let membersAuthInstance;
+let membersAuthEnumerationInstance;
 let userResetInstance;
 let contentApiKeyInstance;
 
@@ -152,6 +153,39 @@ const membersAuth = () => {
     return membersAuthInstance;
 };
 
+/**
+ * This one should have higher limits because it checks across all email addresses
+ */
+const membersAuthEnumeration = () => {
+    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 (!membersAuthEnumerationInstance) {
+        membersAuthEnumerationInstance = new ExpressBrute(store,
+            extend({
+                attachResetToRequest: true,
+                failCallback(req, res, next, nextValidRequestDate) {
+                    return next(new errors.TooManyRequestsError({
+                        message: `Too many different sign-in attempts try again in ${moment(nextValidRequestDate).fromNow(true)}`,
+                        context: tpl(messages.tooManySigninAttempts.context),
+                        help: tpl(messages.tooManySigninAttempts.context)
+                    }));
+                },
+                handleStoreError: handleStoreError
+            }, pick(spamMemberLogin, spamConfigKeys))
+        );
+    }
+
+    return membersAuthEnumerationInstance;
+};
+
 // 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
@@ -281,7 +315,29 @@ module.exports = {
     globalReset: globalReset,
     userLogin: userLogin,
     membersAuth: membersAuth,
+    membersAuthEnumeration: membersAuthEnumeration,
     userReset: userReset,
     privateBlog: privateBlog,
-    contentApiKey: contentApiKey
+    contentApiKey: contentApiKey,
+    reset: () => {
+        store = undefined;
+        memoryStore = undefined;
+        privateBlogInstance = undefined;
+        globalResetInstance = undefined;
+        globalBlockInstance = undefined;
+        userLoginInstance = undefined;
+        membersAuthInstance = undefined;
+        membersAuthEnumerationInstance = undefined;
+        userResetInstance = undefined;
+        contentApiKeyInstance = undefined;
+
+        spam = config.get('spam') || {};
+        spamPrivateBlock = spam.private_block || {};
+        spamGlobalBlock = spam.global_block || {};
+        spamGlobalReset = spam.global_reset || {};
+        spamUserReset = spam.user_reset || {};
+        spamUserLogin = spam.user_login || {};
+        spamMemberLogin = spam.member_login || {};
+        spamContentApiKey = spam.content_api_key || {};
+    }
 };

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

@@ -84,6 +84,7 @@ module.exports = {
     },
 
     /**
+     * Block too many password guesses for the same email address
      */
     membersAuth(req, res, next) {
         return spamPrevention.membersAuth().getMiddleware({
@@ -96,5 +97,12 @@ module.exports = {
                 return _next();
             }
         })(req, res, next);
+    },
+
+    /**
+     * Blocks user enumeration
+     */
+    membersAuthEnumeration(req, res, next) {
+        return spamPrevention.membersAuthEnumeration().prevent(req, res, next);
     }
 };

ghost/core/core/shared/config/defaults.json

@@ -99,6 +99,12 @@
             "maxWait": 86400000,
             "lifetime": 3600,
             "freeRetries": 99
+        },
+        "member_login": {
+            "minWait": 600000,
+            "maxWait": 43200000,
+            "lifetime": 43200,
+            "freeRetries": 8
         }
     },
     "caching": {

ghost/core/test/e2e-api/members/__snapshots__/send-magic-link.test.js.snap

@@ -0,0 +1,55 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`sendMagicLink Throws an error when logging in to a email that does not exist (invite only) 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": null,
+      "details": null,
+      "ghostErrorCode": null,
+      "help": null,
+      "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+      "message": "No member exists with this e-mail address.",
+      "property": null,
+      "type": "BadRequestError",
+    },
+  ],
+}
+`;
+
+exports[`sendMagicLink Throws an error when logging in to a email that does not exist 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": null,
+      "details": null,
+      "ghostErrorCode": null,
+      "help": null,
+      "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+      "message": "No member exists with this e-mail address. Please sign up first.",
+      "property": null,
+      "type": "BadRequestError",
+    },
+  ],
+}
+`;
+
+exports[`sendMagicLink Throws an error when trying to sign up on an invite only site 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": null,
+      "details": null,
+      "ghostErrorCode": null,
+      "help": null,
+      "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+      "message": "This site is invite-only, contact the owner for access.",
+      "property": null,
+      "type": "BadRequestError",
+    },
+  ],
+}
+`;

ghost/core/test/e2e-api/members/send-magic-link.test.js

@@ -1,5 +1,7 @@
-const {agentProvider, mockManager, fixtureManager} = require('../../utils/e2e-framework');
+const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
 const should = require('should');
+const settingsCache = require('../../../core/shared/settings-cache');
+const {anyErrorId} = matchers;
 
 let membersAgent, membersService;
 
@@ -21,6 +23,9 @@ describe('sendMagicLink', function () {
 
     beforeEach(function () {
         mockManager.mockMail();
+
+        // Reset settings
+        settingsCache.set('members_signup_access', {value: 'all'});
     });
 
     afterEach(function () {
@@ -36,6 +41,61 @@ describe('sendMagicLink', function () {
             .expectStatus(400);
     });
     
+    it('Throws an error when logging in to a email that does not exist', async function () {
+        const email = 'this-member-does-not-exist@test.com';
+        await membersAgent.post('/api/send-magic-link')
+            .body({
+                email,
+                emailType: 'signin'
+            })
+            .expectStatus(400)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyErrorId,
+                    // Add this here because it is easy to be overlooked (we need a human readable error!)
+                    // 'Please sign up first' should be included only when invite only is disabled.
+                    message: 'No member exists with this e-mail address. Please sign up first.'
+                }]
+            });
+    });
+
+    it('Throws an error when logging in to a email that does not exist (invite only)', async function () {
+        settingsCache.set('members_signup_access', {value: 'invite'});
+
+        const email = 'this-member-does-not-exist@test.com';
+        await membersAgent.post('/api/send-magic-link')
+            .body({
+                email,
+                emailType: 'signin'
+            })
+            .expectStatus(400)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyErrorId,
+                    // Add this here because it is easy to be overlooked (we need a human readable error!)
+                    // 'Please sign up first' should NOT be included
+                    message: 'No member exists with this e-mail address.'
+                }]
+            });
+    });
+
+    it('Throws an error when trying to sign up on an invite only site', async function () {
+        settingsCache.set('members_signup_access', {value: 'invite'});
+
+        const email = 'this-member-does-not-exist@test.com';
+        await membersAgent.post('/api/send-magic-link')
+            .body({
+                email,
+                emailType: 'signup'
+            })
+            .expectStatus(400)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyErrorId
+                }]
+            });
+    });
+
     it('Creates a valid magic link with tokenData, and without urlHistory', async function () {
         const email = 'newly-created-user-magic-link-test@test.com';
         await membersAgent.post('/api/send-magic-link')

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

@@ -1,8 +1,8 @@
-const {agentProvider, mockManager, fixtureManager, dbUtils, configUtils} = require('../../utils/e2e-framework');
+const {agentProvider, mockManager, fixtureManager, configUtils, resetRateLimits, dbUtils} = require('../../utils/e2e-framework');
 const models = require('../../../core/server/models');
 const assert = require('assert');
 require('should');
-const labsService = require('../../../core/shared/labs');
+const sinon = require('sinon');
 
 let membersAgent, membersService;
 
@@ -17,9 +17,9 @@ async function assertMemberEvents({eventType, memberId, asserts}) {
     assert.equal(events.length, asserts.length, `Only ${asserts.length} ${eventType} should have been added.`);
 }
 
-async function getMemberByEmail(email) {
+async function getMemberByEmail(email, require = true) {
     // eslint-disable-next-line dot-notation
-    return await models['Member'].where('email', email).fetch({require: true});
+    return await models['Member'].where('email', email).fetch({require});
 }
 
 describe('Members Signin', function () {
@@ -48,7 +48,7 @@ describe('Members Signin', function () {
     });
 
     it('Will set a cookie if the token is valid', async function () {
-        const magicLink = await membersService.api.getMagicLink('member1@test.com');
+        const magicLink = await membersService.api.getMagicLink('member1@test.com', 'signup');
         const magicLinkUrl = new URL(magicLink);
         const token = magicLinkUrl.searchParams.get('token');
 
@@ -59,7 +59,7 @@ describe('Members Signin', function () {
     });
 
     it('Will redirect to the free welcome page for signup', async function () {
-        const magicLink = await membersService.api.getMagicLink('member1@test.com');
+        const magicLink = await membersService.api.getMagicLink('member1@test.com', 'signup');
         const magicLinkUrl = new URL(magicLink);
         const token = magicLinkUrl.searchParams.get('token');
 
@@ -70,7 +70,7 @@ describe('Members Signin', function () {
     });
 
     it('Will redirect to the paid welcome page for signup-paid', async function () {
-        const magicLink = await membersService.api.getMagicLink('paid@test.com');
+        const magicLink = await membersService.api.getMagicLink('paid@test.com', 'signup');
         const magicLinkUrl = new URL(magicLink);
         const token = magicLinkUrl.searchParams.get('token');
 
@@ -81,7 +81,7 @@ describe('Members Signin', function () {
     });
 
     it('Will redirect to the free welcome page for subscribe', async function () {
-        const magicLink = await membersService.api.getMagicLink('member1@test.com');
+        const magicLink = await membersService.api.getMagicLink('member1@test.com', 'signup');
         const magicLinkUrl = new URL(magicLink);
         const token = magicLinkUrl.searchParams.get('token');
 
@@ -93,7 +93,7 @@ describe('Members Signin', function () {
 
     it('Will create a new member on signup', async function () {
         const email = 'not-existent-member@test.com';
-        const magicLink = await membersService.api.getMagicLink(email);
+        const magicLink = await membersService.api.getMagicLink(email, 'signup');
         const magicLinkUrl = new URL(magicLink);
         const token = magicLinkUrl.searchParams.get('token');
 
@@ -120,9 +120,194 @@ describe('Members Signin', function () {
         });
     });
 
+    it('Allows a signin via a signup link', async function () {        
+        // This member should be created by the previous test
+        const email = 'not-existent-member@test.com';
+
+        const magicLink = await membersService.api.getMagicLink(email, 'signup');
+        const magicLinkUrl = new URL(magicLink);
+        const token = magicLinkUrl.searchParams.get('token');
+
+        await membersAgent.get(`/?token=${token}&action=signup`)
+            .expectStatus(302)
+            .expectHeader('Location', /\/welcome-free\/$/)
+            .expectHeader('Set-Cookie', /members-ssr.*/);
+    });
+
+    it('Will not create a new member on signin', async function () {
+        const email = 'not-existent-member-2@test.com';
+        const magicLink = await membersService.api.getMagicLink(email, 'signin');
+        const magicLinkUrl = new URL(magicLink);
+        const token = magicLinkUrl.searchParams.get('token');
+
+        // Note: we deliberately set the wrong action here, because this action should be ignored by the backend
+        // and only used by the frontend.
+        await membersAgent.get(`/?token=${token}&action=signup`)
+            .expectStatus(302)
+            .expectHeader('Location', /success=false/);
+
+        const member = await getMemberByEmail(email, false);
+        assert(!member, 'Member should not have been created');
+    });
+
     describe('Rate limiting', function () {
-        it('Will clear rate limits for members auth', async function () {
+        let clock;
+
+        beforeEach(async function () {
             await dbUtils.truncate('brute');
+            await resetRateLimits();
+            clock = sinon.useFakeTimers(new Date());
+        });
+
+        afterEach(function () {
+            clock.restore();
+        });
+
+        it('Will rate limit member enumeration', async function () {
+            // +1 because this is a retry count, so we have one request + the retries, then blocked
+            const userLoginRateLimit = configUtils.config.get('spam').member_login.freeRetries + 1;
+
+            for (let i = 0; i < userLoginRateLimit; i++) {
+                await membersAgent.post('/api/send-magic-link')
+                    .body({
+                        email: 'rate-limiting-test-' + i + '@test.com',
+                        emailType: 'signup'
+                    })
+                    .expectStatus(201);
+            }
+
+            // Now we've been rate limited for every email
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'other@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Now we've been rate limited
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'one@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Get one of the magic link emails
+            const mail = mockManager.assert.sentEmail({
+                to: 'rate-limiting-test-0@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 works, but we're still rate limited (otherwise this would be an easy escape to allow user enumeration)
+            await membersAgent.get(magicLink.pathname + magicLink.search);
+
+            // We are still rate limited
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Wait 10 minutes and check if we are still rate limited
+            clock.tick(10 * 60 * 1000);
+            
+            // We should be able to send a new email
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(201);
+
+            // But only once
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any2@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Waiting 10 minutes is still enough (fibonacci)
+            clock.tick(10 * 60 * 1000);
+
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any2@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(201);
+
+            // Blocked again
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any3@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Waiting 10 minutes is not enough any longer
+            clock.tick(10 * 60 * 1000);
+
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any3@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Waiting 20 minutes is enough
+            clock.tick(10 * 60 * 1000);
+
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any2@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(201);
+
+            // Blocked again
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any3@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(429);
+
+            // Waiting 12 hours is enough to reset it completely
+            clock.tick(12 * 60 * 60 * 1000 + 1000);
+
+            // We can try multiple times again
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any4@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(201);
+
+            // Blocked again
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'any5@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(201);
+        });
+
+        it('Will clear rate limits for members auth', async function () {
+            // Temporary increase the member_login rate limits to a higher number
+            // because other wise we would hit user enumeration rate limits (this won't get reset after a succeeded login)
+            // We need to do this here otherwise the middlewares are not setup correctly
+            configUtils.set('spam:member_login:freeRetries', 40);
+
+            // We need to reset spam instances to apply the configuration change
+            await resetRateLimits();
+
             // +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;
 
@@ -131,13 +316,15 @@ describe('Members Signin', function () {
                     .body({
                         email: 'rate-limiting-test-1@test.com',
                         emailType: 'signup'
-                    });
+                    })
+                    .expectStatus(201);
 
                 await membersAgent.post('/api/send-magic-link')
                     .body({
                         email: 'rate-limiting-test-2@test.com',
                         emailType: 'signup'
-                    });
+                    })
+                    .expectStatus(201);
             }
 
             // Now we've been rate limited
@@ -186,13 +373,24 @@ describe('Members Signin', function () {
                     emailType: 'signup'
                 })
                 .expectStatus(429);
+
+            // Wait 10 minutes and check if we are still rate limited
+            clock.tick(10 * 60 * 1000);
+            
+            // We should be able to send a new email
+            await membersAgent.post('/api/send-magic-link')
+                .body({
+                    email: 'rate-limiting-test-2@test.com',
+                    emailType: 'signup'
+                })
+                .expectStatus(201);
         });
     });
 
     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';
-            const magicLink = await membersService.api.getMagicLink(email, {
+            const magicLink = await membersService.api.getMagicLink(email, 'signup', {
                 attribution: {
                     id: 'test_source_id',
                     url: '/test-source-url/',

ghost/core/test/e2e-frontend/members.test.js

@@ -24,11 +24,14 @@ describe('Front-end members behavior', function () {
     let request;
 
     async function loginAsMember(email) {
+        // Member should exist, because we are signin in
+        await models.Member.findOne({email}, {require: true});
+
         // membersService needs to be required after Ghost start so that settings
         // are pre-populated with defaults
         const membersService = require('../../core/server/services/members');
 
-        const signinLink = await membersService.api.getMagicLink(email);
+        const signinLink = await membersService.api.getMagicLink(email, 'signin');
         const signinURL = new URL(signinLink);
         // request needs a relative path rather than full url with host
         const signinPath = `${signinURL.pathname}${signinURL.search}`;
@@ -411,11 +414,14 @@ describe('Front-end members behavior', function () {
         describe('as paid member', function () {
             const email = 'paid@test.com';
             before(async function () {
+                // Member should exist, because we are signin in
+                await models.Member.findOne({email}, {require: true});
+
                 // membersService needs to be required after Ghost start so that settings
                 // are pre-populated with defaults
                 const membersService = require('../../core/server/services/members');
 
-                const signinLink = await membersService.api.getMagicLink(email);
+                const signinLink = await membersService.api.getMagicLink(email, 'signin');
                 const signinURL = new URL(signinLink);
                 // request needs a relative path rather than full url with host
                 const signinPath = `${signinURL.pathname}${signinURL.search}`;

ghost/core/test/utils/agents/members-api-test-agent.js

@@ -18,7 +18,7 @@ class MembersAPITestAgent extends TestAgent {
 
     async loginAs(email) {
         const membersService = require('../../../core/server/services/members');
-        const magicLink = await membersService.api.getMagicLink(email);
+        const magicLink = await membersService.api.getMagicLink(email, 'signup');
         const magicLinkUrl = new URL(magicLink);
         const token = magicLinkUrl.searchParams.get('token');
 

ghost/core/test/utils/e2e-framework.js

@@ -128,6 +128,15 @@ const getFixture = (type, index = 0) => {
     return fixtureUtils.DataGenerator.forKnex[type][index];
 };
 
+/**
+ * Reset rate limit instances (not the brute table)
+ */
+const resetRateLimits = async () => {
+    // Reset rate limiting instances
+    const {spamPrevention} = require('../../core/server/web/shared/middleware/api');
+    spamPrevention.reset();
+};
+
 /**
  * This function ensures that Ghost's data is reset back to "factory settings"
  *
@@ -140,6 +149,9 @@ const resetData = async () => {
 
     // Clear out the database
     await db.reset({truncate: true});
+
+    // Reset rate limiting instances (resetting the table is not enough!)
+    await resetRateLimits();
 };
 
 /**
@@ -374,5 +386,6 @@ module.exports = {
     // utilities
     configUtils: require('./configUtils'),
     dbUtils: require('./db-utils'),
-    urlUtils: require('./urlUtils')
+    urlUtils: require('./urlUtils'),
+    resetRateLimits
 };

ghost/magic-link/lib/MagicLink.js

@@ -83,13 +83,12 @@ class MagicLink {
      *
      * @param {object} options
      * @param {TokenData} options.tokenData - The data for token
-     * @param {string=} [options.type='signin'] - The type to be passed to the url and content generator functions
+     * @param {string=} [options.type='signin'] - The type to be passed to the url and content generator functions. This type will also get stored in the token data.
      * @returns {Promise<URL>} - signin URL
      */
     async getMagicLink(options) {
-        const token = await this.tokenProvider.create(options.tokenData);
-
-        const type = options.type || 'signin';
+        const type = options.type ?? 'signin';
+        const token = await this.tokenProvider.create({...options.tokenData, type});
 
         return this.getSigninURL(token, type);
     }

ghost/members-api/lib/MembersAPI.js

@@ -192,11 +192,21 @@ module.exports = function MembersAPI({
                 type = 'signup';
             }
         }
-        return magicLinkService.sendMagicLink({email, type, tokenData: Object.assign({email}, tokenData), referrer});
+        return magicLinkService.sendMagicLink({email, type, tokenData: Object.assign({email, type}, tokenData), referrer});
     }
 
-    function getMagicLink(email, tokenData = {}) {
-        return magicLinkService.getMagicLink({tokenData: {email, ...tokenData}, type: 'signin'});
+    /**
+     * 
+     * @param {string} email 
+     * @param {'signin'|'signup'} type When you specify 'signin' this will prevent the creation of a new member if no member is found with the provided email
+     * @param {*} [tokenData] Optional token data to add to the token
+     * @returns 
+     */
+    function getMagicLink(email, type, tokenData = {}) {
+        return magicLinkService.getMagicLink({
+            tokenData: {email, ...tokenData}, 
+            type
+        });
     }
 
     async function getTokenDataFromMagicLinkToken(token) {
@@ -204,7 +214,7 @@ module.exports = function MembersAPI({
     }
 
     async function getMemberDataFromMagicLinkToken(token) {
-        const {email, labels = [], name = '', oldEmail, newsletters, attribution, reqIp} = await getTokenDataFromMagicLinkToken(token);
+        const {email, labels = [], name = '', oldEmail, newsletters, attribution, reqIp, type} = await getTokenDataFromMagicLinkToken(token);
         if (!email) {
             return null;
         }
@@ -213,7 +223,7 @@ module.exports = function MembersAPI({
 
         if (member) {
             await MemberLoginEvent.add({member_id: member.id});
-            if (oldEmail) {
+            if (oldEmail && (!type || type === 'updateEmail')) {
                 // user exists but wants to change their email address
                 await users.update({email}, {id: member.id});
                 return getMemberIdentityData(email);
@@ -221,6 +231,13 @@ module.exports = function MembersAPI({
             return member;
         }
 
+        // Note: old tokens can still have a missing type (we can remove this after a couple of weeks)
+        if (type && !['signup', 'subscribe'].includes(type)) {
+            // Don't allow sign up
+            // Note that we use the type from inside the magic token so this behaviour can't be changed
+            return null;
+        }
+
         let geolocation;
         if (reqIp) {
             try {

ghost/members-api/lib/controllers/router.js

@@ -2,14 +2,19 @@ const tpl = require('@tryghost/tpl');
 const logging = require('@tryghost/logging');
 const _ = require('lodash');
 const {BadRequestError, NoPermissionError, NotFoundError, UnauthorizedError} = require('@tryghost/errors');
+const errors = require('@tryghost/errors');
 
 const messages = {
+    emailRequired: 'Email is required.',
     badRequest: 'Bad Request.',
     notFound: 'Not Found.',
     offerArchived: 'This offer is archived.',
     tierArchived: 'This tier is archived.',
     existingSubscription: 'A subscription exists for this Member.',
-    unableToCheckout: 'Unable to initiate checkout session'
+    unableToCheckout: 'Unable to initiate checkout session',
+    inviteOnly: 'This site is invite-only, contact the owner for access.',
+    memberNotFound: 'No member exists with this e-mail address.',
+    memberNotFoundSignUp: 'No member exists with this e-mail address. Please sign up first.'
 };
 
 module.exports = class RouterController {
@@ -401,24 +406,43 @@ module.exports = class RouterController {
     }
 
     async sendMagicLink(req, res) {
-        const {email, emailType, autoRedirect} = req.body;
+        const {email, autoRedirect} = req.body;
+        let {emailType} = req.body;
+
         let referer = req.get('referer');
         if (autoRedirect === false){
             referer = null;
         }
         if (!email) {
+            throw new errors.BadRequestError({
+                message: tpl(messages.emailRequired)
+            });
+        }
+
+        if (!emailType) {
+            // Default to subscribe form that also allows to login (safe fallback for older clients)
+            if (!this._allowSelfSignup()) {
+                emailType = 'signin';
+            } else {
+                emailType = 'subscribe';
+            }
+        }
+
+        if (!['signin', 'signup', 'subscribe'].includes(emailType)) {
             res.writeHead(400);
             return res.end('Bad Request.');
         }
 
         try {
+            if (emailType === 'signup' || emailType === 'subscribe') {
                 if (!this._allowSelfSignup()) {
-                const member = await this._memberRepository.get({email});
-                if (member) {
-                    const tokenData = {};
-                    await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, referrer: referer});
+                    throw new errors.BadRequestError({
+                        message: tpl(messages.inviteOnly)
+                    });
                 }
-            } else {
+
+                // Someone tries to signup with a user that already exists
+                // -> doesn't really matter: we'll send a login link
                 const tokenData = _.pick(req.body, ['labels', 'name', 'newsletters']);
                 if (req.ip) {
                     tokenData.reqIp = req.ip;
@@ -427,19 +451,32 @@ module.exports = class RouterController {
                 tokenData.attribution = await this._memberAttributionService.getAttribution(req.body.urlHistory);
 
                 await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, referrer: referer});
-            }
+                
                 res.writeHead(201);
                 return res.end('Created.');
+            }
+
+            // Signin
+            const member = await this._memberRepository.get({email});
+            if (member) {
+                const tokenData = {};
+                await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, referrer: referer});
+                res.writeHead(201);
+                return res.end('Created.');
+            }
+            
+            throw new errors.BadRequestError({
+                message: this._allowSelfSignup() ? tpl(messages.memberNotFoundSignUp) : tpl(messages.memberNotFound)
+            });
         } catch (err) {
             if (err.code === 'EENVELOPE') {
                 logging.error(err);
                 res.writeHead(400);
                 return res.end('Bad Request.');
             }
-            const statusCode = (err && err.statusCode) || 500;
-            logging.error(err);
-            res.writeHead(statusCode);
-            return res.end('Internal Server Error.');
+
+            // Let the normal error middleware handle this error
+            throw err;
         }
     }
 };

ghost/members-ssr/lib/MembersSSR.js

@@ -217,6 +217,13 @@ class MembersSSR {
         const token = Array.isArray(query.token) ? query.token[0] : query.token;
         const member = await this._getMemberDataFromToken(token);
 
+        if (!member) {
+            // The member doesn't exist any longer (could be a sign in token for a member that was deleted)
+            return Promise.reject(new BadRequestError({
+                message: 'Invalid token'
+            }));
+        }
+
         // perform and store geoip lookup for members when they log in
         if (!member.geolocation) {
             try {