From 6957c2725b89a110c2077d26cfc4a336bb2e7147 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 17 Sep 2020 15:42:01 +0100 Subject: [PATCH] Refactored magic-link to be more generic (#202) no-issue This removes the concept of `subject` & `payload` from the function signatures, making the implementation a little more generic, and less JWT centric. We also replace getUserFromToken and getPayloadFromToken with a single method getDataFromToken, which will contain all the necessary data. * Updated members-api to use new magic-link module This updates the usage of magic-link to work with the new interface * Fixed labels not saving for new members Due to how bookshelf-relations works, we must fetch the labels before saving a member, otherwise the labels are all deleted. * Used a proper class rather than constructor function This just moves the code to a more modern standard * Updated methods to be async This prepares us for a future where token generation and validation may require access to storage and thus be an asyncronous operation --- ghost/magic-link/README.md | 6 +- ghost/magic-link/index.js | 192 +++++++++++++--------------- ghost/magic-link/test/index.test.js | 14 +- ghost/members-api/index.js | 40 +++--- 4 files changed, 118 insertions(+), 134 deletions(-) diff --git a/ghost/magic-link/README.md b/ghost/magic-link/README.md index 76ccd8f6f4..2b2d6dbfe5 100644 --- a/ghost/magic-link/README.md +++ b/ghost/magic-link/README.md @@ -59,9 +59,9 @@ async function main() { /** * POST /signin */ - const {token, info} = await service.sendMagicLink({ + const {url, info} = await service.sendMagicLink({ email: 'test@example.com', - user: { + tokenData: { id: 'some-id' } }); @@ -74,7 +74,7 @@ async function main() { /** * GET /signin */ - const user = await service.getUserFromToken(token); + const data = await service.getDataFromToken(token); // createSomeKindOfSession(user); } diff --git a/ghost/magic-link/index.js b/ghost/magic-link/index.js index 350b8501d6..542a3fa0c0 100644 --- a/ghost/magic-link/index.js +++ b/ghost/magic-link/index.js @@ -1,5 +1,4 @@ const jwt = require('jsonwebtoken'); -module.exports = MagicLink; /** * @typedef { import('jsonwebtoken').Secret } Secret @@ -9,6 +8,92 @@ module.exports = MagicLink; * @typedef { string } URL */ +class MagicLink { + /** + * @param {object} options + * @param {MailTransporter} options.transporter + * @param {Secret} options.secret + * @param {(token: JSONWebToken, type: string) => URL} options.getSigninURL + * @param {typeof defaultGetText} [options.getText] + * @param {typeof defaultGetHTML} [options.getHTML] + * @param {typeof defaultGetSubject} [options.getSubject] + */ + constructor(options) { + if (!options || !options.transporter || !options.secret || !options.getSigninURL) { + throw new Error('Missing options. Expects {transporter, secret, getSigninURL}'); + } + this.transporter = options.transporter; + this.secret = options.secret; + this.getSigninURL = options.getSigninURL; + this.getText = options.getText || defaultGetText; + this.getHTML = options.getHTML || defaultGetHTML; + this.getSubject = options.getSubject || defaultGetSubject; + } + + /** + * sendMagicLink + * + * @param {object} options + * @param {string} options.email - The email to send magic link to + * @param {object} options.tokenData - The data for token + * @param {string=} [options.type='signin'] - The type to be passed to the url and content generator functions + * @returns {Promise<{token: JSONWebToken, info: SentMessageInfo}>} + */ + async sendMagicLink(options) { + const token = jwt.sign(options.tokenData, this.secret, { + algorithm: 'HS256', + expiresIn: '10m' + }); + + const type = options.type || 'signin'; + + const url = this.getSigninURL(token, type); + + const info = await this.transporter.sendMail({ + to: options.email, + subject: this.getSubject(type), + text: this.getText(url, type, options.email), + html: this.getHTML(url, type, options.email) + }); + + return {token, info}; + } + + /** + * getMagicLink + * + * @param {object} options + * @param {object} options.tokenData - The data for token + * @param {string=} [options.type='signin'] - The type to be passed to the url and content generator functions + * @returns {Promise} - signin URL + */ + async getMagicLink(options) { + const token = jwt.sign(options.tokenData, this.secret, { + algorithm: 'HS256', + expiresIn: '10m' + }); + + const type = options.type || 'signin'; + + return this.getSigninURL(token, type); + } + + /** + * getDataFromToken + * + * @param {JSONWebToken} token - The token to decode + * @returns {Promise} data - The data object associated with the magic link + */ + async getDataFromToken(token) { + /** @type {object} */ + const tokenData = (jwt.verify(token, this.secret, { + algorithms: ['HS256'], + maxAge: '10m' + })); + return tokenData; + } +} + /** * defaultGetText * @@ -54,107 +139,4 @@ function defaultGetSubject(type) { return `Signin!`; } -/** - * MagicLink - * @constructor - * - * @param {object} options - * @param {MailTransporter} options.transporter - * @param {Secret} options.secret - * @param {(token: JSONWebToken, type: string) => URL} options.getSigninURL - * @param {typeof defaultGetText} [options.getText] - * @param {typeof defaultGetHTML} [options.getHTML] - * @param {typeof defaultGetSubject} [options.getSubject] - */ -function MagicLink(options) { - if (!options || !options.transporter || !options.secret || !options.getSigninURL) { - throw new Error('Missing options. Expects {transporter, secret, getSigninURL}'); - } - this.transporter = options.transporter; - this.secret = options.secret; - this.getSigninURL = options.getSigninURL; - this.getText = options.getText || defaultGetText; - this.getHTML = options.getHTML || defaultGetHTML; - this.getSubject = options.getSubject || defaultGetSubject; -} - -/** - * sendMagicLink - * - * @param {object} options - * @param {string} options.email - The email to send magic link to - * @param {string} options.payload - The payload for token - * @param {object} options.subject - The subject to associate with the magic link (user id, or email) - * @param {string=} [options.type='signin'] - The type to be passed to the url and content generator functions - * @returns {Promise<{token: JSONWebToken, info: SentMessageInfo}>} - */ -MagicLink.prototype.sendMagicLink = async function sendMagicLink(options) { - const payload = options.payload || {}; - const token = jwt.sign(payload, this.secret, { - algorithm: 'HS256', - subject: options.subject, - expiresIn: '10m' - }); - - const type = options.type || 'signin'; - - const url = this.getSigninURL(token, type); - - const info = await this.transporter.sendMail({ - to: options.email, - subject: this.getSubject(type), - text: this.getText(url, type, options.email), - html: this.getHTML(url, type, options.email) - }); - - return {token, info}; -}; -/** - * getMagicLink - * - * @param {object} options - * @param {object} options.subject - The subject to associate with the magic link (user id, or email) - * @param {string=} [options.type='signin'] - The type to be passed to the url and content generator functions - * @returns {string} - signin URL - */ -MagicLink.prototype.getMagicLink = function getMagicLink(options) { - const token = jwt.sign({}, this.secret, { - algorithm: 'HS256', - subject: options.subject, - expiresIn: '10m' - }); - - const type = options.type || 'signin'; - - return this.getSigninURL(token, type); -}; - -/** - * getUserFromToken - * - * @param {JSONWebToken} token - The token to decode - * @returns {object} user - The user object associated with the magic link - */ -MagicLink.prototype.getUserFromToken = function getUserFromToken(token) { - /** @type {object} */ - const claims = jwt.verify(token, this.secret, { - algorithms: ['HS256'], - maxAge: '10m' - }); - return claims.sub; -}; - -/** - * getPayloadFromToken - * - * @param {JSONWebToken} token - The token to decode - * @returns {object} payload - The payload object associated with the magic link - */ -MagicLink.prototype.getPayloadFromToken = function getPayloadFromToken(token) { - /** @type {object} */ - const claims = jwt.verify(token, this.secret, { - algorithms: ['HS256'], - maxAge: '10m' - }); - return claims || {}; -}; +module.exports = MagicLink; diff --git a/ghost/magic-link/test/index.test.js b/ghost/magic-link/test/index.test.js index 204018d070..eba3f36fa1 100644 --- a/ghost/magic-link/test/index.test.js +++ b/ghost/magic-link/test/index.test.js @@ -27,7 +27,9 @@ describe('MagicLink', function () { const args = { email: 'test@example.com', - subject: '420', + tokenData: { + id: '420' + }, type: 'blazeit' }; const {token} = await service.sendMagicLink(args); @@ -52,7 +54,7 @@ describe('MagicLink', function () { }); }); - describe('#getUserFromToken', function () { + describe('#getDataFromToken', function () { it('Returns the user data which from the token that was encoded by #sendMagicLink', async function () { const options = { secret, @@ -67,13 +69,15 @@ describe('MagicLink', function () { const args = { email: 'test@example.com', - subject: '420' + tokenData: { + id: '420' + } }; const {token} = await service.sendMagicLink(args); - const subject = service.getUserFromToken(token); + const data = await service.getDataFromToken(token); - should.deepEqual(subject, args.subject); + should.deepEqual(data.id, args.tokenData.id); }); }); }); diff --git a/ghost/members-api/index.js b/ghost/members-api/index.js index 37268bb347..209813d22d 100644 --- a/ghost/members-api/index.js +++ b/ghost/members-api/index.js @@ -105,17 +105,17 @@ module.exports = function MembersApi({ Member }); - async function sendEmailWithMagicLink({email, requestedType, payload, options = {forceEmailType: false}}) { - if (options.forceEmailType) { - return magicLinkService.sendMagicLink({email, payload, subject: email, type: requestedType}); - } - const member = await users.get({email}); - if (member) { - return magicLinkService.sendMagicLink({email, payload, subject: email, type: 'signin'}); - } else { - const type = requestedType === 'subscribe' ? 'subscribe' : 'signup'; - return magicLinkService.sendMagicLink({email, payload, subject: email, type}); + async function sendEmailWithMagicLink({email, requestedType, tokenData, options = {forceEmailType: false}}) { + let type = requestedType; + if (!options.forceEmailType) { + const member = await users.get({email}); + if (member) { + type = 'signin'; + } else if (type !== 'subscribe') { + type = 'signup'; + } } + return magicLinkService.sendMagicLink({email, type, tokenData: Object.assign({email}, tokenData)}); } function getMagicLink(email) { @@ -123,8 +123,7 @@ module.exports = function MembersApi({ } async function getMemberDataFromMagicLinkToken(token) { - const email = await magicLinkService.getUserFromToken(token); - const {labels = [], name = '', oldEmail} = await magicLinkService.getPayloadFromToken(token); + const {email, labels = [], name = '', oldEmail} = await magicLinkService.getDataFromToken(token); if (!email) { return null; } @@ -170,7 +169,9 @@ module.exports = function MembersApi({ })); } - const member = await getMemberIdentityData(email); + const member = await users.get(email, { + withRelated: ['labels'] + }); if (!member) { return Promise.reject(new common.errors.NotFoundError({ @@ -198,7 +199,6 @@ module.exports = function MembersApi({ middleware.sendMagicLink.use(body.json(), async function (req, res) { const {email, emailType, oldEmail} = req.body; - const payload = {}; if (!email) { res.writeHead(400); @@ -214,18 +214,16 @@ module.exports = function MembersApi({ }); } } - let extraPayload = {}; + if (!allowSelfSignup) { const member = await users.get({email}); if (member) { - extraPayload = _.pick(req.body, ['oldEmail']); - Object.assign(payload, extraPayload); - await sendEmailWithMagicLink({email, requestedType: emailType, payload}); + const tokenData = _.pick(req.body, ['oldEmail']); + await sendEmailWithMagicLink({email, tokenData, requestedType: emailType}); } } else { - extraPayload = _.pick(req.body, ['labels', 'name', 'oldEmail']); - Object.assign(payload, extraPayload); - await sendEmailWithMagicLink({email, requestedType: emailType, payload}); + const tokenData = _.pick(req.body, ['labels', 'name', 'oldEmail']); + await sendEmailWithMagicLink({email, tokenData, requestedType: emailType}); } res.writeHead(201); return res.end('Created.');