mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-02-03 23:00:14 -05:00
Fixed security hole in email address change flow
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-65p7-pjj8-ggmr The email address change flow was built on top of the unauthenticated signin/signup flow. This meant that ownership of the email being changed wasn't verified and allowed a malicious actore to change the email address of arbitrary accounts to an email address which they controlled. We remove the ability to change email addresses from the signin/signup flow and instead create a dedicated, authenticated flow for changing email address.
This commit is contained in:
parent
21fbaff41b
commit
4e947a88ce
3 changed files with 57 additions and 32 deletions
|
@ -156,8 +156,9 @@ module.exports = function MembersAPI({
|
|||
const memberController = new MemberController({
|
||||
memberRepository,
|
||||
StripePrice,
|
||||
stripeAPIService,
|
||||
tokenService
|
||||
tokenService,
|
||||
sendEmailWithMagicLink,
|
||||
allowSelfSignup
|
||||
});
|
||||
|
||||
const routerController = new RouterController({
|
||||
|
@ -356,6 +357,10 @@ module.exports = function MembersAPI({
|
|||
body.json(),
|
||||
(req, res) => MembersAnalyticsIngress.createEvents(req, res)
|
||||
),
|
||||
updateEmailAddress: Router().use(
|
||||
body.json(),
|
||||
(req, res) => memberController.updateEmailAddress(req, res)
|
||||
),
|
||||
updateSubscription: Router({mergeParams: true}).use(
|
||||
body.json(),
|
||||
(req, res) => memberController.updateSubscription(req, res)
|
||||
|
|
|
@ -1,25 +1,57 @@
|
|||
const errors = require('@tryghost/ignition-errors');
|
||||
|
||||
module.exports = class MemberController {
|
||||
/**
|
||||
* MemberController
|
||||
*
|
||||
* @param {object} deps
|
||||
* @param {any} deps.memberRepository
|
||||
* @param {any} deps.StripePrice
|
||||
* @param {any} deps.stripeApiService
|
||||
* @param {any} deps.tokenService
|
||||
* @param {any} deps.sendEmailWithMagicLink
|
||||
* @param {boolean} deps.allowSelfSignup
|
||||
*/
|
||||
module.exports = class MemberController {
|
||||
constructor({
|
||||
memberRepository,
|
||||
StripePrice,
|
||||
stripeAPIService,
|
||||
tokenService
|
||||
tokenService,
|
||||
sendEmailWithMagicLink,
|
||||
allowSelfSignup
|
||||
}) {
|
||||
this._memberRepository = memberRepository;
|
||||
this._StripePrice = StripePrice;
|
||||
this._stripeApiService = stripeAPIService;
|
||||
this._tokenService = tokenService;
|
||||
this._sendEmailWithMagicLink = sendEmailWithMagicLink;
|
||||
this._allowSelfSignup = allowSelfSignup;
|
||||
}
|
||||
|
||||
async updateEmailAddress(req, res) {
|
||||
const identity = req.body.identity;
|
||||
const email = req.body.email;
|
||||
const options = {
|
||||
forceEmailType: true
|
||||
};
|
||||
|
||||
if (!identity) {
|
||||
res.writeHead(403);
|
||||
return res.end('No Permission.');
|
||||
}
|
||||
|
||||
let tokenData = {};
|
||||
try {
|
||||
const member = await this._memberRepository.getByToken(identity);
|
||||
tokenData.oldEmail = member.get('email');
|
||||
} catch (err) {
|
||||
res.writeHead(401);
|
||||
res.end('Unauthorized.');
|
||||
}
|
||||
|
||||
try {
|
||||
await this._sendEmailWithMagicLink({email, tokenData, requestedType: 'updateEmail', options});
|
||||
res.writeHead(201);
|
||||
return res.end('Created.');
|
||||
} catch (err) {
|
||||
res.writeHead(500);
|
||||
return res.end('Internal Server Error.');
|
||||
}
|
||||
}
|
||||
|
||||
async updateSubscription(req, res) {
|
||||
|
|
|
@ -1,6 +1,5 @@
|
|||
const common = require('../../lib/common');
|
||||
const _ = require('lodash');
|
||||
const errors = require('@tryghost/ignition-errors');
|
||||
|
||||
/**
|
||||
* RouterController
|
||||
|
@ -219,33 +218,22 @@ module.exports = class RouterController {
|
|||
}
|
||||
|
||||
async sendMagicLink(req, res) {
|
||||
const {email, emailType, oldEmail, requestSrc} = req.body;
|
||||
let forceEmailType = false;
|
||||
const {email, emailType, requestSrc} = req.body;
|
||||
if (!email) {
|
||||
res.writeHead(400);
|
||||
return res.end('Bad Request.');
|
||||
}
|
||||
|
||||
try {
|
||||
if (oldEmail) {
|
||||
const existingMember = await this._memberRepository.get({email});
|
||||
if (existingMember) {
|
||||
throw new errors.BadRequestError({
|
||||
message: 'This email is already associated with a member'
|
||||
});
|
||||
}
|
||||
forceEmailType = true;
|
||||
}
|
||||
|
||||
if (!this._allowSelfSignup) {
|
||||
const member = oldEmail ? await this._memberRepository.get({oldEmail}) : await this._memberRepository.get({email});
|
||||
const member = await this._memberRepository.get({email});
|
||||
if (member) {
|
||||
const tokenData = _.pick(req.body, ['oldEmail']);
|
||||
await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, requestSrc, options: {forceEmailType}});
|
||||
const tokenData = {};
|
||||
await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, requestSrc});
|
||||
}
|
||||
} else {
|
||||
const tokenData = _.pick(req.body, ['labels', 'name', 'oldEmail']);
|
||||
await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, requestSrc, options: {forceEmailType}});
|
||||
const tokenData = _.pick(req.body, ['labels', 'name']);
|
||||
await this._sendEmailWithMagicLink({email, tokenData, requestedType: emailType, requestSrc});
|
||||
}
|
||||
res.writeHead(201);
|
||||
return res.end('Created.');
|
||||
|
|
Loading…
Add table
Reference in a new issue