🔒 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.
This commit is contained in:
Simon Backx 2022-10-05 12:42:42 +02:00 committed by GitHub
parent 609fcb17c0
commit e7378520a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
25 changed files with 636 additions and 77 deletions

32
errors.js Normal file
View File

@ -0,0 +1,32 @@
export class HumanReadableError extends Error {
/**
* Returns whether this response from the server is a human readable error and should be shown to the user.
* @param {Response} res
* @returns {HumanReadableError|undefined}
*/
static async fromApiResponse(res) {
// Bad request + Too many requests
if (res.status === 400 || res.status === 429) {
try {
const json = await res.json();
if (json.errors && Array.isArray(json.errors) && json.errors.length > 0 && json.errors[0].message) {
return new HumanReadableError(json.errors[0].message);
}
} catch (e) {
// Failed to decode: ignore
return false;
}
}
}
/**
* Only output the message of an error if it is a human readable error and should be exposed to the user.
* Otherwise it returns a default generic message.
*/
static getMessageFromError(error, defaultMessage) {
if (error instanceof HumanReadableError) {
return error.message;
}
return defaultMessage;
}
}

View File

@ -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'),

View File

@ -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;

View File

@ -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));

View File

@ -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 || {};
}
};

View File

@ -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);
}
};

View File

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

View File

@ -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",
},
],
}
`;

View File

@ -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,12 +23,70 @@ describe('sendMagicLink', function () {
beforeEach(function () {
mockManager.mockMail();
// Reset settings
settingsCache.set('members_signup_access', {value: 'all'});
});
afterEach(function () {
mockManager.restore();
});
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')

View File

@ -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/',

View File

@ -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}`;

View File

@ -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');

View File

@ -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();
};
/**
@ -378,5 +390,6 @@ module.exports = {
// utilities
configUtils: require('./configUtils'),
dbUtils: require('./db-utils'),
urlUtils: require('./urlUtils')
urlUtils: require('./urlUtils'),
resetRateLimits
};

View File

@ -73,13 +73,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);
}

View File

@ -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 {

View File

@ -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;
}
}
};

View File

@ -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 {

View File

@ -1,3 +1,4 @@
import {HumanReadableError} from './utils/errors';
import {createPopupNotification, getMemberEmail, getMemberName, getProductCadenceFromPrice, removePortalLinkFromUrl} from './utils/helpers';
function switchPage({data, state}) {
@ -77,7 +78,7 @@ async function signout({api, state}) {
async function signin({data, api, state}) {
try {
await api.member.sendMagicLink(data);
await api.member.sendMagicLink({...data, emailType: 'signin'});
return {
page: 'magiclink',
lastPage: 'signin'
@ -87,7 +88,7 @@ async function signin({data, api, state}) {
action: 'signin:failed',
popupNotification: createPopupNotification({
type: 'signin:failed', autoHide: false, closeable: true, state, status: 'error',
message: 'Failed to log in, please try again'
message: HumanReadableError.getMessageFromError(e, 'Failed to log in, please try again')
})
};
}
@ -97,7 +98,7 @@ async function signup({data, state, api}) {
try {
let {plan, tierId, cadence, email, name, newsletters, offerId} = data;
if (plan.toLowerCase() === 'free') {
await api.member.sendMagicLink(data);
await api.member.sendMagicLink({emailType: 'signup', ...data});
} else {
if (tierId && cadence) {
await api.member.checkoutPlan({plan, tierId, cadence, email, name, newsletters, offerId});

View File

@ -32,7 +32,7 @@ export default class SigninPage extends React.Component {
return {
errors: ValidateInputForm({fields: this.getInputFields({state})})
};
}, () => {
}, async () => {
const {email, errors} = this.state;
const hasFormErrors = (errors && Object.values(errors).filter(d => !!d).length > 0);
if (!hasFormErrors) {

View File

@ -1,6 +1,6 @@
/* eslint-disable no-console */
import {getQueryPrice, getUrlHistory} from './utils/helpers';
import {HumanReadableError} from './utils/errors';
export function formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}) {
form.removeEventListener('submit', submitHandler);
@ -51,11 +51,16 @@ export function formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}
if (res.ok) {
form.classList.add('success');
} else {
return HumanReadableError.fromApiResponse(res).then((e) => {
throw e;
});
}
}).catch((err) => {
if (errorEl) {
errorEl.innerText = 'There was an error sending the email, please try again';
// This theme supports a custom error element
errorEl.innerText = HumanReadableError.getMessageFromError(err, 'There was an error sending the email, please try again');
}
form.classList.add('error');
}
});
}

View File

@ -141,7 +141,8 @@ describe('Signin', () => {
fireEvent.click(submitButton);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com'
email: 'jamie@example.com',
emailType: 'signin'
});
const magicLink = await within(popupIframeDocument).findByText(/sent you a login link/i);
@ -166,7 +167,8 @@ describe('Signin', () => {
fireEvent.click(submitButton);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com'
email: 'jamie@example.com',
emailType: 'signin'
});
const magicLink = await within(popupIframeDocument).findByText(/sent you a login link/i);
@ -191,7 +193,8 @@ describe('Signin', () => {
fireEvent.click(submitButton);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com'
email: 'jamie@example.com',
emailType: 'signin'
});
const magicLink = await within(popupIframeDocument).findByText(/sent you a login link/i);
@ -230,7 +233,8 @@ describe('Signin', () => {
fireEvent.click(submitButton);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com'
email: 'jamie@example.com',
emailType: 'signin'
});
const magicLink = await within(popupIframeDocument).findByText(/sent you a login link/i);
@ -255,7 +259,8 @@ describe('Signin', () => {
fireEvent.click(submitButton);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com'
email: 'jamie@example.com',
emailType: 'signin'
});
const magicLink = await within(popupIframeDocument).findByText(/sent you a login link/i);
@ -280,7 +285,8 @@ describe('Signin', () => {
fireEvent.click(submitButton);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com'
email: 'jamie@example.com',
emailType: 'signin'
});
const magicLink = await within(popupIframeDocument).findByText(/sent you a login link/i);

View File

@ -208,6 +208,7 @@ describe('Signup', () => {
fireEvent.click(chooseBtns[0]);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com',
emailType: 'signup',
name: 'Jamie Larsen',
plan: 'free'
});
@ -242,6 +243,7 @@ describe('Signup', () => {
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com',
emailType: 'signup',
name: '',
plan: 'free'
});
@ -281,6 +283,7 @@ describe('Signup', () => {
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com',
emailType: 'signup',
name: 'Jamie Larsen',
plan: 'free'
});
@ -566,6 +569,7 @@ describe('Signup', () => {
fireEvent.click(chooseBtns[0]);
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com',
emailType: 'signup',
name: 'Jamie Larsen',
plan: 'free'
});
@ -596,6 +600,7 @@ describe('Signup', () => {
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com',
emailType: 'signup',
name: '',
plan: 'free'
});
@ -632,6 +637,7 @@ describe('Signup', () => {
expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({
email: 'jamie@example.com',
emailType: 'signup',
name: 'Jamie Larsen',
plan: 'free'
});

View File

@ -1,3 +1,4 @@
import {HumanReadableError} from './errors';
import {transformApiSiteData, transformApiTiersData, getUrlHistory} from './helpers';
function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) {
@ -192,7 +193,7 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) {
});
},
sendMagicLink({email, emailType, labels, name, oldEmail, newsletters}) {
async sendMagicLink({email, emailType, labels, name, oldEmail, newsletters}) {
const url = endpointFor({type: 'members', resource: 'send-magic-link'});
const body = {
name,
@ -208,20 +209,25 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) {
body.urlHistory = urlHistory;
}
return makeRequest({
const res = await makeRequest({
url,
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify(body)
}).then(function (res) {
});
if (res.ok) {
return 'Success';
} else {
// Try to read body error message that is human readable and should be shown to the user
const humanError = await HumanReadableError.fromApiResponse(res);
if (humanError) {
throw humanError;
}
throw new Error('Failed to send magic link email');
}
});
},
signout() {

View File

@ -0,0 +1,32 @@
export class HumanReadableError extends Error {
/**
* Returns whether this response from the server is a human readable error and should be shown to the user.
* @param {Response} res
* @returns {HumanReadableError|undefined}
*/
static async fromApiResponse(res) {
// Bad request + Too many requests
if (res.status === 400 || res.status === 429) {
try {
const json = await res.json();
if (json.errors && Array.isArray(json.errors) && json.errors.length > 0 && json.errors[0].message) {
return new HumanReadableError(json.errors[0].message);
}
} catch (e) {
// Failed to decode: ignore
return false;
}
}
}
/**
* Only output the message of an error if it is a human readable error and should be exposed to the user.
* Otherwise it returns a default generic message.
*/
static getMessageFromError(error, defaultMessage) {
if (error instanceof HumanReadableError) {
return error.message;
}
return defaultMessage;
}
}

View File

@ -15,7 +15,7 @@ const setupProvider = (context) => {
};
const customRender = (ui, {options = {}, overrideContext = {}} = {}) => {
const mockOnActionFn = jest.fn();
const mockOnActionFn = jest.fn().mockResolvedValue(undefined);
const context = {
site: testSite,