From 55d5a8d892da58b474f66f0cfb61988d390f003c Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Mon, 24 Jul 2023 16:01:02 +0200 Subject: [PATCH 1/3] Fixed loading assets from CDN URL refs https://ghost.slack.com/archives/C027S85FS/p1690202522054729 - this is another set of places where we load assets slightly differently - this should fix user profile images when using assets from a CDN --- ghost/admin/app/components/modal-post-history.js | 2 +- .../modals/newsletters/edit/preview-labs.js | 3 ++- .../components/modals/newsletters/edit/preview.js | 3 ++- ghost/admin/app/helpers/parse-history-event.js | 14 ++++++-------- ghost/admin/app/models/user.js | 5 +++-- ghost/admin/app/utils/ghost-paths.js | 8 +------- 6 files changed, 15 insertions(+), 20 deletions(-) diff --git a/ghost/admin/app/components/modal-post-history.js b/ghost/admin/app/components/modal-post-history.js index 7e22998d8a..54b546ed63 100644 --- a/ghost/admin/app/components/modal-post-history.js +++ b/ghost/admin/app/components/modal-post-history.js @@ -55,7 +55,7 @@ export default class ModalPostHistory extends Component { feature_image_caption: revision.get('featureImageCaption'), author: { name: revision.get('author.name') || 'Deleted staff user', - profile_image_url: revision.get('author.profileImage') || this.ghostPaths.assetRoot.replace(/\/$/, '') + '/img/user-image.png' + profile_image_url: revision.get('author.profileImageUrl') }, postStatus: revision.get('postStatus'), reason: revision.get('reason'), diff --git a/ghost/admin/app/components/modals/newsletters/edit/preview-labs.js b/ghost/admin/app/components/modals/newsletters/edit/preview-labs.js index 67b0cf16ad..df1bdbc66f 100644 --- a/ghost/admin/app/components/modals/newsletters/edit/preview-labs.js +++ b/ghost/admin/app/components/modals/newsletters/edit/preview-labs.js @@ -1,4 +1,5 @@ import Component from '@glimmer/component'; +import config from 'ghost-admin/config/environment'; import moment from 'moment-timezone'; import {htmlSafe} from '@ember/template'; import {inject as service} from '@ember/service'; @@ -119,7 +120,7 @@ export default class EditNewsletterPreview extends Component { get featureImageUrl() { // keep path separate so asset rewriting correctly picks it up const imagePath = '/img/user-cover.png'; - const fullPath = this.ghostPaths.assetRoot.replace(/\/$/, '') + imagePath; + const fullPath = (config.cdnUrl ? '' : this.ghostPaths.assetRoot.replace(/\/$/, '')) + imagePath; return fullPath; } diff --git a/ghost/admin/app/components/modals/newsletters/edit/preview.js b/ghost/admin/app/components/modals/newsletters/edit/preview.js index a7172332b8..bfcded1689 100644 --- a/ghost/admin/app/components/modals/newsletters/edit/preview.js +++ b/ghost/admin/app/components/modals/newsletters/edit/preview.js @@ -1,4 +1,5 @@ import Component from '@glimmer/component'; +import config from 'ghost-admin/config/environment'; import moment from 'moment-timezone'; import {htmlSafe} from '@ember/template'; import {inject as service} from '@ember/service'; @@ -43,7 +44,7 @@ export default class EditNewsletterPreview extends Component { get featureImageUrl() { // keep path separate so asset rewriting correctly picks it up const imagePath = '/img/user-cover.png'; - const fullPath = this.ghostPaths.assetRoot.replace(/\/$/, '') + imagePath; + const fullPath = (config.cdnUrl ? '' : this.ghostPaths.assetRoot.replace(/\/$/, '')) + imagePath; return fullPath; } diff --git a/ghost/admin/app/helpers/parse-history-event.js b/ghost/admin/app/helpers/parse-history-event.js index af887eea93..8328e91402 100644 --- a/ghost/admin/app/helpers/parse-history-event.js +++ b/ghost/admin/app/helpers/parse-history-event.js @@ -1,4 +1,5 @@ import Helper from '@ember/component/helper'; +import config from 'ghost-admin/config/environment'; import {inject as service} from '@ember/service'; export default class ParseHistoryEvent extends Helper { @@ -13,7 +14,7 @@ export default class ParseHistoryEvent extends Helper { const actor = getActor(ev); const actorLinkTarget = getActorLinkTarget(ev); - const assetRoot = this.ghostPaths.assetRoot.replace(/\/$/, ''); + const assetRoot = (config.cdnUrl ? '' : this.ghostPaths.assetRoot.replace(/\/$/, '')); const actorIcon = getActorIcon(ev, assetRoot); return { @@ -39,14 +40,11 @@ function getActor(ev) { } function getActorIcon(ev, assetRoot) { - const defaultImage = `${assetRoot}/img/user-image.png`; + const defaultImage = `/img/user-image.png`; + let defaultImageUrl = `${assetRoot}${defaultImage}`; - if (!ev.actor.id) { - return defaultImage; - } - - if (!ev.actor.image) { - return defaultImage; + if (!ev.actor.id || !ev.actor.image) { + return defaultImageUrl; } return ev.actor.image; diff --git a/ghost/admin/app/models/user.js b/ghost/admin/app/models/user.js index 07cf77fe30..c794b4cd0d 100644 --- a/ghost/admin/app/models/user.js +++ b/ghost/admin/app/models/user.js @@ -1,6 +1,7 @@ /* eslint-disable camelcase */ import BaseModel from './base'; import ValidationEngine from 'ghost-admin/mixins/validation-engine'; +import config from 'ghost-admin/config/environment'; import {attr, hasMany} from '@ember-data/model'; import {computed} from '@ember/object'; import {equal, or} from '@ember/object/computed'; @@ -89,14 +90,14 @@ export default BaseModel.extend(ValidationEngine, { profileImageUrl: computed('ghostPaths.assetRoot', 'profileImage', function () { // keep path separate so asset rewriting correctly picks it up let defaultImage = '/img/user-image.png'; - let defaultPath = this.ghostPaths.assetRoot.replace(/\/$/, '') + defaultImage; + let defaultPath = (config.cdnUrl ? '' : this.ghostPaths.assetRoot.replace(/\/$/, '')) + defaultImage; return this.profileImage || defaultPath; }), coverImageUrl: computed('ghostPaths.assetRoot', 'coverImage', function () { // keep path separate so asset rewriting correctly picks it up let defaultImage = '/img/user-cover.png'; - let defaultPath = this.ghostPaths.assetRoot.replace(/\/$/, '') + defaultImage; + let defaultPath = (config.cdnUrl ? '' : this.ghostPaths.assetRoot.replace(/\/$/, '')) + defaultImage; return this.coverImage || defaultPath; }), diff --git a/ghost/admin/app/utils/ghost-paths.js b/ghost/admin/app/utils/ghost-paths.js index e07b3ac8fa..1fad93d628 100644 --- a/ghost/admin/app/utils/ghost-paths.js +++ b/ghost/admin/app/utils/ghost-paths.js @@ -20,10 +20,6 @@ export default function () { let assetRoot = `${subdir}/ghost/assets/`; let apiRoot = `${subdir}/ghost/api/admin`; - function assetUrl(src) { - return subdir + src; - } - return { adminRoot, assetRoot, @@ -48,9 +44,7 @@ export default function () { return arg.slice(-1) === '/' ? arg : `${arg}/`; } return '/'; - }, - - asset: assetUrl + } } }; } From abc7af8082ebf6b1230f5cb70e886418a3e17403 Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Thu, 27 Jul 2023 08:46:50 +0200 Subject: [PATCH 2/3] Added test email rate limiting (#17505) refs https://github.com/TryGhost/Product/issues/3651 - This is a security fix that addresses an issue causing malicious users to abuse the test / preview email API endpoint. - We have multiple procedures in place now to limit such users. - First, we now only allow one email address to be passed into the `sendTestEmail` method. This method only have one purpose, which is to compliment the test email functionality within the Editor in Admin and therefore have no reason to send to more than one email address at a time. - We then add an additional rate limiter to prevent a user from making multiple requests, eg via a script. - The new imposed limit is 10 test emails per hour. --- .../server/web/api/endpoints/admin/routes.js | 3 +- .../shared/middleware/api/spam-prevention.js | 32 +++++++++++- .../server/web/shared/middleware/brute.js | 13 +++++ ghost/core/core/shared/config/defaults.json | 6 +++ .../config/env/config.testing-browser.json | 6 +++ .../config/env/config.testing-mysql.json | 7 ++- .../shared/config/env/config.testing.json | 6 +++ .../admin/email-preview-rate-limiter.test.js | 51 +++++++++++++++++++ ghost/email-service/lib/EmailController.js | 10 +++- .../test/email-controller.test.js | 28 ++++++++++ 10 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js diff --git a/ghost/core/core/server/web/api/endpoints/admin/routes.js b/ghost/core/core/server/web/api/endpoints/admin/routes.js index 7896f599ec..2a387f800d 100644 --- a/ghost/core/core/server/web/api/endpoints/admin/routes.js +++ b/ghost/core/core/server/web/api/endpoints/admin/routes.js @@ -313,7 +313,8 @@ module.exports = function apiRoutes() { // ## Email Preview router.get('/email_previews/posts/:id', mw.authAdminApi, http(api.email_previews.read)); - router.post('/email_previews/posts/:id', mw.authAdminApi, http(api.email_previews.sendTestEmail)); + // preview sending have an additional rate limiter to prevent abuse + router.post('/email_previews/posts/:id', shared.middleware.brute.previewEmailLimiter, mw.authAdminApi, http(api.email_previews.sendTestEmail)); // ## Emails router.get('/emails', mw.authAdminApi, http(api.emails.browse)); diff --git a/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js b/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js index d47490538b..90248b730a 100644 --- a/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js +++ b/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js @@ -21,7 +21,8 @@ const messages = { context: 'Too many login attempts.' }, tooManyAttempts: 'Too many attempts.', - webmentionsBlock: 'Too many mention attempts' + webmentionsBlock: 'Too many mention attempts', + emailPreviewBlock: 'Only 10 test emails can be sent per hour' }; let spamPrivateBlock = spam.private_block || {}; let spamGlobalBlock = spam.global_block || {}; @@ -31,6 +32,7 @@ let spamUserLogin = spam.user_login || {}; let spamMemberLogin = spam.member_login || {}; let spamContentApiKey = spam.content_api_key || {}; let spamWebmentionsBlock = spam.webmentions_block || {}; +let spamEmailPreviewBlock = spam.email_preview_block || {}; let store; let memoryStore; @@ -43,6 +45,7 @@ let membersAuthInstance; let membersAuthEnumerationInstance; let userResetInstance; let contentApiKeyInstance; +let emailPreviewBlockInstance; const spamConfigKeys = ['freeRetries', 'minWait', 'maxWait', 'lifetime']; @@ -152,6 +155,32 @@ const webmentionsBlock = () => { return webmentionsBlockInstance; }; +const emailPreviewBlock = () => { + 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 + }); + + emailPreviewBlockInstance = emailPreviewBlockInstance || new ExpressBrute(store, + extend({ + attachResetToRequest: false, + failCallback(req, res, next) { + return next(new errors.TooManyRequestsError({ + message: messages.emailPreviewBlock + })); + }, + handleStoreError: handleStoreError + }, pick(spamEmailPreviewBlock, spamConfigKeys)) + ); + + return emailPreviewBlockInstance; +}; + const membersAuth = () => { const ExpressBrute = require('express-brute'); const BruteKnex = require('brute-knex'); @@ -349,6 +378,7 @@ module.exports = { privateBlog: privateBlog, contentApiKey: contentApiKey, webmentionsBlock: webmentionsBlock, + emailPreviewBlock: emailPreviewBlock, reset: () => { store = undefined; memoryStore = undefined; diff --git a/ghost/core/core/server/web/shared/middleware/brute.js b/ghost/core/core/server/web/shared/middleware/brute.js index 496132888c..f64ad6f8ed 100644 --- a/ghost/core/core/server/web/shared/middleware/brute.js +++ b/ghost/core/core/server/web/shared/middleware/brute.js @@ -117,5 +117,18 @@ module.exports = { return _next('webmention_blocked'); } })(req, res, next); + }, + + /** + * Blocks preview email spam + */ + + previewEmailLimiter(req, res, next) { + return spamPrevention.emailPreviewBlock().getMiddleware({ + ignoreIP: false, + key(_req, _res, _next) { + return _next('preview_email_blocked'); + } + })(req, res, next); } }; diff --git a/ghost/core/core/shared/config/defaults.json b/ghost/core/core/shared/config/defaults.json index 191ddc4f78..853120745a 100644 --- a/ghost/core/core/shared/config/defaults.json +++ b/ghost/core/core/shared/config/defaults.json @@ -108,6 +108,12 @@ "maxWait": 100, "lifetime": 1000, "freeRetries": 100 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } }, "caching": { diff --git a/ghost/core/core/shared/config/env/config.testing-browser.json b/ghost/core/core/shared/config/env/config.testing-browser.json index e534e72d0d..ee2ed00386 100644 --- a/ghost/core/core/shared/config/env/config.testing-browser.json +++ b/ghost/core/core/shared/config/env/config.testing-browser.json @@ -49,6 +49,12 @@ "maxWait": 100000, "lifetime": 3600, "freeRetries": 3 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } }, "privacy": { diff --git a/ghost/core/core/shared/config/env/config.testing-mysql.json b/ghost/core/core/shared/config/env/config.testing-mysql.json index 0f4ea5acf2..3c15227b1c 100644 --- a/ghost/core/core/shared/config/env/config.testing-mysql.json +++ b/ghost/core/core/shared/config/env/config.testing-mysql.json @@ -50,8 +50,13 @@ "maxWait": 100000, "lifetime": 3600, "freeRetries": 3 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } - }, "privacy": { "useTinfoil": true, diff --git a/ghost/core/core/shared/config/env/config.testing.json b/ghost/core/core/shared/config/env/config.testing.json index 84fcbd6b58..74e2f1ec78 100644 --- a/ghost/core/core/shared/config/env/config.testing.json +++ b/ghost/core/core/shared/config/env/config.testing.json @@ -49,6 +49,12 @@ "maxWait": 100000, "lifetime": 3600, "freeRetries": 3 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } }, "privacy": { diff --git a/ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js b/ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js new file mode 100644 index 0000000000..8893b4bf45 --- /dev/null +++ b/ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js @@ -0,0 +1,51 @@ +// Decided to have this test separately from the other email preview tests since the rate limiter would interfere with the other tests + +const {agentProvider, fixtureManager, mockManager, configUtils} = require('../../utils/e2e-framework'); +const sinon = require('sinon'); +const DomainEvents = require('@tryghost/domain-events'); + +async function allSettled() { + await DomainEvents.allSettled(); +} + +describe('Rate limiter', function () { + let agent; + + afterEach(function () { + mockManager.restore(); + sinon.restore(); + }); + + beforeEach(function () { + mockManager.mockMailgun(); + }); + + before(async function () { + agent = await agentProvider.getAdminAPIAgent(); + await fixtureManager.init('users', 'newsletters', 'posts'); + await agent.loginAsOwner(); + }); + + it('is rate limited against spammmer requests', async function () { + const testEmailSpamBlock = configUtils.config.get('spam').email_preview_block; + const requests = []; + for (let i = 0; i < testEmailSpamBlock.freeRetries + 1; i += 1) { + const req = await agent + .post(`email_previews/posts/${fixtureManager.get('posts', 0).id}/`) + .body({ + emails: ['test@ghost.org'] + }); + requests.push(req); + } + await Promise.all(requests); + + await agent + .post(`email_previews/posts/${fixtureManager.get('posts', 0).id}/`) + .body({ + emails: ['test@ghost.org'] + }) + .expectStatus(429); + + await allSettled(); + }); +}); diff --git a/ghost/email-service/lib/EmailController.js b/ghost/email-service/lib/EmailController.js index 2b85bc8b1b..6a17be0006 100644 --- a/ghost/email-service/lib/EmailController.js +++ b/ghost/email-service/lib/EmailController.js @@ -4,7 +4,8 @@ const tpl = require('@tryghost/tpl'); const messages = { postNotFound: 'Post not found.', noEmailsProvided: 'No emails provided.', - emailNotFound: 'Email not found.' + emailNotFound: 'Email not found.', + tooManyEmailsProvided: 'Too many emails provided. Maximum of 1 test email can be sent at once.' }; class EmailController { @@ -67,6 +68,13 @@ class EmailController { }); } + // test emails are limited to 1 + if (emails.length > 1) { + throw new errors.ValidationError({ + message: tpl(messages.tooManyEmailsProvided) + }); + } + await this.service.sendTestEmail(post, newsletter, segment, emails); } diff --git a/ghost/email-service/test/email-controller.test.js b/ghost/email-service/test/email-controller.test.js index 3538c7fda2..b0d2977103 100644 --- a/ghost/email-service/test/email-controller.test.js +++ b/ghost/email-service/test/email-controller.test.js @@ -230,6 +230,34 @@ describe('Email Controller', function () { }); assert.equal(result, undefined); }); + + it('throw if more than one email is provided', async function () { + const service = { + sendTestEmail: () => { + return Promise.resolve({id: 'mail@id'}); + } + }; + + const controller = new EmailController(service, { + models: { + Post: createModelClass({ + findOne: { + title: 'Post title' + } + }), + Newsletter: createModelClass() + } + }); + + await assert.rejects(controller.sendTestEmail({ + options: {}, + data: { + id: '123', + newsletter: 'newsletter-slug', + emails: ['example@example.com', 'example2@example.com'] + } + }), /Too many emails provided. Maximum of 1 test email can be sent at once./); + }); }); describe('retryFailedEmail', function () { From 5273b56e88dc92218d846c7a299ecb01bdd92500 Mon Sep 17 00:00:00 2001 From: Ghost CI <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 27 Jul 2023 07:38:58 +0000 Subject: [PATCH 3/3] v5.55.2 --- ghost/admin/package.json | 2 +- ghost/core/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ghost/admin/package.json b/ghost/admin/package.json index f096abb8f9..ff76a2550a 100644 --- a/ghost/admin/package.json +++ b/ghost/admin/package.json @@ -1,6 +1,6 @@ { "name": "ghost-admin", - "version": "5.55.1", + "version": "5.55.2", "description": "Ember.js admin client for Ghost", "author": "Ghost Foundation", "homepage": "http://ghost.org", diff --git a/ghost/core/package.json b/ghost/core/package.json index 58b1928f97..abcb398bb0 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "5.55.1", + "version": "5.55.2", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org",