From ef4f79370fbbd01fe6c36f463c5557fc9cd969d6 Mon Sep 17 00:00:00 2001 From: Sam Lord Date: Thu, 22 Aug 2024 11:34:33 +0100 Subject: [PATCH] Added support in Portal for integrity tokens on magic link API ref KTLO-1 These tokens should prevent untargeted attacks, as the magic link endpoint needs a token that was generated by the server, similar to a CSRF token, but without needing any server-side state, or a cookie to be set for unauthenticated users. --- apps/portal/src/actions.js | 6 +- apps/portal/src/data-attributes.js | 21 ++++-- apps/portal/src/tests/SigninFlow.test.js | 68 +++++++++++------ apps/portal/src/tests/SignupFlow.test.js | 75 ++++++++++++------- apps/portal/src/tests/data-attributes.test.js | 40 ++++++---- apps/portal/src/utils/api.js | 17 ++++- .../server/services/members/middleware.js | 2 +- 7 files changed, 153 insertions(+), 76 deletions(-) diff --git a/apps/portal/src/actions.js b/apps/portal/src/actions.js index 686927e062..b0a7d4c641 100644 --- a/apps/portal/src/actions.js +++ b/apps/portal/src/actions.js @@ -79,7 +79,8 @@ async function signout({api, state}) { async function signin({data, api, state}) { try { - await api.member.sendMagicLink({...data, emailType: 'signin'}); + const integrityToken = await api.member.getIntegrityToken(); + await api.member.sendMagicLink({...data, emailType: 'signin', integrityToken}); return { page: 'magiclink', lastPage: 'signin' @@ -100,7 +101,8 @@ async function signup({data, state, api}) { let {plan, tierId, cadence, email, name, newsletters, offerId} = data; if (plan.toLowerCase() === 'free') { - await api.member.sendMagicLink({emailType: 'signup', ...data}); + const integrityToken = await api.member.getIntegrityToken(); + await api.member.sendMagicLink({emailType: 'signup', integrityToken, ...data}); } else { if (tierId && cadence) { await api.member.checkoutPlan({plan, tierId, cadence, email, name, newsletters, offerId}); diff --git a/apps/portal/src/data-attributes.js b/apps/portal/src/data-attributes.js index 7d80798633..9f4e9daefe 100644 --- a/apps/portal/src/data-attributes.js +++ b/apps/portal/src/data-attributes.js @@ -56,12 +56,21 @@ export function formSubmitHandler({event, form, errorEl, siteUrl, submitHandler} } } - fetch(`${siteUrl}/members/api/send-magic-link/`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json' - }, - body: JSON.stringify(reqBody) + return fetch(`${siteUrl}/members/api/integrity-token/`, { + method: 'GET' + }).then((res) => { + return res.text(); + }).then((integrityToken) => { + return fetch(`${siteUrl}/members/api/send-magic-link/`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + ...reqBody, + integrityToken + }) + }); }).then(function (res) { form.addEventListener('submit', submitHandler); form.classList.remove('loading'); diff --git a/apps/portal/src/tests/SigninFlow.test.js b/apps/portal/src/tests/SigninFlow.test.js index 5186965b78..5a75d29ac6 100644 --- a/apps/portal/src/tests/SigninFlow.test.js +++ b/apps/portal/src/tests/SigninFlow.test.js @@ -16,6 +16,10 @@ const setup = async ({site, member = null}) => { return Promise.resolve('success'); }); + ghostApi.member.getIntegrityToken = jest.fn(() => { + return Promise.resolve('testtoken'); + }); + ghostApi.member.checkoutPlan = jest.fn(() => { return Promise.resolve(); }); @@ -67,6 +71,10 @@ const multiTierSetup = async ({site, member = null}) => { return Promise.resolve('success'); }); + ghostApi.member.getIntegrityToken = jest.fn(() => { + return Promise.resolve(`testtoken`); + }); + ghostApi.member.checkoutPlan = jest.fn(() => { return Promise.resolve(); }); @@ -139,13 +147,15 @@ describe('Signin', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(submitButton); - expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ - email: 'jamie@example.com', - emailType: 'signin' - }); const magicLink = await within(popupIframeDocument).findByText(/Now check your email/i); expect(magicLink).toBeInTheDocument(); + + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ + email: 'jamie@example.com', + emailType: 'signin', + integrityToken: 'testtoken' + }); }); test('without name field', async () => { @@ -165,13 +175,15 @@ describe('Signin', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(submitButton); - expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ - email: 'jamie@example.com', - emailType: 'signin' - }); const magicLink = await within(popupIframeDocument).findByText(/Now check your email/i); expect(magicLink).toBeInTheDocument(); + + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ + email: 'jamie@example.com', + emailType: 'signin', + integrityToken: 'testtoken' + }); }); test('with only free plan', async () => { @@ -191,13 +203,15 @@ describe('Signin', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(submitButton); - expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ - email: 'jamie@example.com', - emailType: 'signin' - }); const magicLink = await within(popupIframeDocument).findByText(/Now check your email/i); expect(magicLink).toBeInTheDocument(); + + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ + email: 'jamie@example.com', + emailType: 'signin', + integrityToken: 'testtoken' + }); }); }); }); @@ -231,13 +245,15 @@ describe('Signin', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(submitButton); - expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ - email: 'jamie@example.com', - emailType: 'signin' - }); const magicLink = await within(popupIframeDocument).findByText(/Now check your email/i); expect(magicLink).toBeInTheDocument(); + + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ + email: 'jamie@example.com', + emailType: 'signin', + integrityToken: 'testtoken' + }); }); test('without name field', async () => { @@ -257,13 +273,15 @@ describe('Signin', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(submitButton); - expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ - email: 'jamie@example.com', - emailType: 'signin' - }); const magicLink = await within(popupIframeDocument).findByText(/Now check your email/i); expect(magicLink).toBeInTheDocument(); + + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ + email: 'jamie@example.com', + emailType: 'signin', + integrityToken: 'testtoken' + }); }); test('with only free plan available', async () => { @@ -283,13 +301,15 @@ describe('Signin', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(submitButton); - expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ - email: 'jamie@example.com', - emailType: 'signin' - }); const magicLink = await within(popupIframeDocument).findByText(/Now check your email/i); expect(magicLink).toBeInTheDocument(); + + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ + email: 'jamie@example.com', + emailType: 'signin', + integrityToken: 'testtoken' + }); }); }); }); diff --git a/apps/portal/src/tests/SignupFlow.test.js b/apps/portal/src/tests/SignupFlow.test.js index dfb587cc39..c293cf73d4 100644 --- a/apps/portal/src/tests/SignupFlow.test.js +++ b/apps/portal/src/tests/SignupFlow.test.js @@ -16,6 +16,10 @@ const offerSetup = async ({site, member = null, offer}) => { return Promise.resolve('success'); }); + ghostApi.member.getIntegrityToken = jest.fn(() => { + return Promise.resolve(`testtoken`); + }); + ghostApi.site.offer = jest.fn(() => { return Promise.resolve({ offers: [offer] @@ -80,6 +84,10 @@ const setup = async ({site, member = null}) => { return Promise.resolve('success'); }); + ghostApi.member.getIntegrityToken = jest.fn(() => { + return Promise.resolve(`testtoken`); + }); + ghostApi.member.checkoutPlan = jest.fn(() => { return Promise.resolve(); }); @@ -133,6 +141,10 @@ const multiTierSetup = async ({site, member = null}) => { return Promise.resolve('success'); }); + ghostApi.member.getIntegrityToken = jest.fn(() => { + return Promise.resolve(`testtoken`); + }); + ghostApi.member.checkoutPlan = jest.fn(() => { return Promise.resolve(); }); @@ -205,14 +217,17 @@ describe('Signup', () => { expect(emailInput).toHaveValue('jamie@example.com'); expect(nameInput).toHaveValue('Jamie Larsen'); fireEvent.click(chooseBtns[0]); + + const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); + expect(magicLink).toBeInTheDocument(); + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ email: 'jamie@example.com', emailType: 'signup', name: 'Jamie Larsen', - plan: 'free' + plan: 'free', + integrityToken: 'testtoken' }); - const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); - expect(magicLink).toBeInTheDocument(); }); test('without name field', async () => { @@ -240,16 +255,17 @@ describe('Signup', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(chooseBtns[0]); + // Check if magic link page is shown + const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); + expect(magicLink).toBeInTheDocument(); + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ email: 'jamie@example.com', emailType: 'signup', name: '', - plan: 'free' + plan: 'free', + integrityToken: 'testtoken' }); - - // Check if magic link page is shown - const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); - expect(magicLink).toBeInTheDocument(); }); test('with only free plan', async () => { @@ -288,16 +304,17 @@ describe('Signup', () => { expect(nameInput).toHaveValue('Jamie Larsen'); fireEvent.click(submitButton); + // Check if magic link page is shown + const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); + expect(magicLink).toBeInTheDocument(); + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ email: 'jamie@example.com', emailType: 'signup', name: 'Jamie Larsen', - plan: 'free' + plan: 'free', + integrityToken: 'testtoken' }); - - // Check if magic link page is shown - const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); - expect(magicLink).toBeInTheDocument(); }); }); @@ -570,14 +587,17 @@ describe('Signup', () => { expect(emailInput).toHaveValue('jamie@example.com'); expect(nameInput).toHaveValue('Jamie Larsen'); fireEvent.click(chooseBtns[0]); + + const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); + expect(magicLink).toBeInTheDocument(); + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ email: 'jamie@example.com', emailType: 'signup', name: 'Jamie Larsen', - plan: 'free' + plan: 'free', + integrityToken: 'testtoken' }); - const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); - expect(magicLink).toBeInTheDocument(); }); test('without name field', async () => { @@ -601,16 +621,17 @@ describe('Signup', () => { expect(emailInput).toHaveValue('jamie@example.com'); fireEvent.click(chooseBtns[0]); + // Check if magic link page is shown + const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); + expect(magicLink).toBeInTheDocument(); + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ email: 'jamie@example.com', emailType: 'signup', name: '', - plan: 'free' + plan: 'free', + integrityToken: 'testtoken' }); - - // Check if magic link page is shown - const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); - expect(magicLink).toBeInTheDocument(); }); test('with only free plan available', async () => { @@ -646,16 +667,17 @@ describe('Signup', () => { expect(nameInput).toHaveValue('Jamie Larsen'); fireEvent.click(submitButton); + // Check if magic link page is shown + const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); + expect(magicLink).toBeInTheDocument(); + expect(ghostApi.member.sendMagicLink).toHaveBeenLastCalledWith({ email: 'jamie@example.com', emailType: 'signup', name: 'Jamie Larsen', - plan: 'free' + plan: 'free', + integrityToken: 'testtoken' }); - - // Check if magic link page is shown - const magicLink = await within(popupIframeDocument).findByText(/now check your email/i); - expect(magicLink).toBeInTheDocument(); }); test('should not show free plan if it is hidden', async () => { @@ -799,4 +821,3 @@ describe('Signup', () => { }); }); }); - diff --git a/apps/portal/src/tests/data-attributes.test.js b/apps/portal/src/tests/data-attributes.test.js index 689f538040..0a0ec95f55 100644 --- a/apps/portal/src/tests/data-attributes.test.js +++ b/apps/portal/src/tests/data-attributes.test.js @@ -86,6 +86,13 @@ describe('Member Data attributes:', () => { }); } + if (url.includes('api/integrity-token')) { + return Promise.resolve({ + ok: true, + text: async () => 'testtoken' + }); + } + if (url.includes('api/session')) { return Promise.resolve({ ok: true, @@ -139,12 +146,12 @@ describe('Member Data attributes:', () => { jest.restoreAllMocks(); }); describe('data-members-form', () => { - test('allows free signup', () => { + test('allows free signup', async () => { const {event, form, errorEl, siteUrl, submitHandler} = getMockData(); - formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}); + await formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}); - expect(window.fetch).toHaveBeenCalledTimes(1); + expect(window.fetch).toHaveBeenCalledTimes(2); const expectedBody = JSON.stringify({ email: 'jamie@example.com', emailType: 'signup', @@ -157,9 +164,10 @@ describe('Member Data attributes:', () => { refSource: 'ghost-explore', refUrl: 'https://example.com/blog/', time: 1611234567890 - }] + }], + integrityToken: 'testtoken' }); - expect(window.fetch).toHaveBeenCalledWith('https://portal.localhost/members/api/send-magic-link/', {body: expectedBody, headers: {'Content-Type': 'application/json'}, method: 'POST'}); + expect(window.fetch).toHaveBeenLastCalledWith('https://portal.localhost/members/api/send-magic-link/', {body: expectedBody, headers: {'Content-Type': 'application/json'}, method: 'POST'}); }); }); @@ -241,16 +249,16 @@ describe('Member Data attributes:', () => { }); describe('data-members-newsletter', () => { - test('includes specified newsletters in request', () => { + test('includes specified newsletters in request', async () => { const {event, form, errorEl, siteUrl, submitHandler} = getMockData({ newsletterQuerySelectorResult: [{ value: 'Some Newsletter' }] }); - formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}); + await formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}); - expect(window.fetch).toHaveBeenCalledTimes(1); + expect(window.fetch).toHaveBeenCalledTimes(2); const expectedBody = JSON.stringify({ email: 'jamie@example.com', emailType: 'signup', @@ -264,19 +272,20 @@ describe('Member Data attributes:', () => { refUrl: 'https://example.com/blog/', time: 1611234567890 }], - newsletters: [{name: 'Some Newsletter'}] + newsletters: [{name: 'Some Newsletter'}], + integrityToken: 'testtoken' }); - expect(window.fetch).toHaveBeenCalledWith('https://portal.localhost/members/api/send-magic-link/', {body: expectedBody, headers: {'Content-Type': 'application/json'}, method: 'POST'}); + expect(window.fetch).toHaveBeenLastCalledWith('https://portal.localhost/members/api/send-magic-link/', {body: expectedBody, headers: {'Content-Type': 'application/json'}, method: 'POST'}); }); - test('does not include newsletters in request if there are no newsletter inputs', () => { + test('does not include newsletters in request if there are no newsletter inputs', async () => { const {event, form, errorEl, siteUrl, submitHandler} = getMockData({ newsletterQuerySelectorResult: [] }); - formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}); + await formSubmitHandler({event, form, errorEl, siteUrl, submitHandler}); - expect(window.fetch).toHaveBeenCalledTimes(1); + expect(window.fetch).toHaveBeenCalledTimes(2); const expectedBody = JSON.stringify({ email: 'jamie@example.com', emailType: 'signup', @@ -289,9 +298,10 @@ describe('Member Data attributes:', () => { refSource: 'ghost-explore', refUrl: 'https://example.com/blog/', time: 1611234567890 - }] + }], + integrityToken: 'testtoken' }); - expect(window.fetch).toHaveBeenCalledWith('https://portal.localhost/members/api/send-magic-link/', {body: expectedBody, headers: {'Content-Type': 'application/json'}, method: 'POST'}); + expect(window.fetch).toHaveBeenLastCalledWith('https://portal.localhost/members/api/send-magic-link/', {body: expectedBody, headers: {'Content-Type': 'application/json'}, method: 'POST'}); }); }); }); diff --git a/apps/portal/src/utils/api.js b/apps/portal/src/utils/api.js index 1fe06f485e..773df5c9ed 100644 --- a/apps/portal/src/utils/api.js +++ b/apps/portal/src/utils/api.js @@ -244,7 +244,21 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { }); }, - async sendMagicLink({email, emailType, labels, name, oldEmail, newsletters, redirect, customUrlHistory, autoRedirect = true}) { + async getIntegrityToken() { + const url = endpointFor({type: 'members', resource: 'integrity-token'}); + const res = await makeRequest({ + url, + method: 'GET' + }); + + if (res.ok) { + return res.text(); + } else { + throw new Error('Failed to start a members session'); + } + }, + + async sendMagicLink({email, emailType, labels, name, oldEmail, newsletters, redirect, integrityToken, customUrlHistory, autoRedirect = true}) { const url = endpointFor({type: 'members', resource: 'send-magic-link'}); const body = { name, @@ -255,6 +269,7 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { labels, requestSrc: 'portal', redirect, + integrityToken, autoRedirect }; const urlHistory = customUrlHistory ?? getUrlHistory(); diff --git a/ghost/core/core/server/services/members/middleware.js b/ghost/core/core/server/services/members/middleware.js index 76d64cb5ec..ffc3ce54dc 100644 --- a/ghost/core/core/server/services/members/middleware.js +++ b/ghost/core/core/server/services/members/middleware.js @@ -170,7 +170,7 @@ const createIntegrityToken = async function createIntegrityToken(req, res) { const verifyIntegrityToken = async function verifyIntegrityToken(req, res, next) { try { - const token = req.query.requestIntegrityToken; + const token = req.body.integrityToken; if (!token) { logging.warn('Request with missing integrity token.'); // In future this will throw an error