diff --git a/.gitignore b/.gitignore index bf60f70f64..dbe454cae8 100644 --- a/.gitignore +++ b/.gitignore @@ -124,6 +124,8 @@ test/functional/*.png /ghost/core/core/frontend/public/ghost.min.css /ghost/core/core/frontend/public/comment-counts.min.js /ghost/core/core/frontend/public/member-attribution.min.js +/ghost/core/core/frontend/public/admin-auth/admin-auth.min.js + # Caddyfile - for local development with ssl + caddy Caddyfile diff --git a/ghost/core/core/bridge.js b/ghost/core/core/bridge.js index 8dd3cf0001..e13f61e095 100644 --- a/ghost/core/core/bridge.js +++ b/ghost/core/core/bridge.js @@ -16,7 +16,7 @@ const logging = require('@tryghost/logging'); const tpl = require('@tryghost/tpl'); const themeEngine = require('./frontend/services/theme-engine'); const appService = require('./frontend/services/apps'); -const {adminAuthAssets, cardAssets} = require('./frontend/services/assets-minification'); +const {cardAssets} = require('./frontend/services/assets-minification'); const routerManager = require('./frontend/services/routing').routerManager; const settingsCache = require('./shared/settings-cache'); const urlService = require('./server/services/url'); @@ -51,10 +51,6 @@ class Bridge { return themeEngine.getActive(); } - ensureAdminAuthAssetsMiddleware() { - return adminAuthAssets.serveMiddleware(); - } - async activateTheme(loadedTheme, checkedTheme) { let settings = { locale: settingsCache.get('locale') @@ -69,10 +65,6 @@ class Bridge { const cardAssetConfig = this.getCardAssetConfig(); debug('reload card assets config', cardAssetConfig); cardAssets.invalidate(cardAssetConfig); - - // TODO: is this in the right place? - // rebuild asset files - adminAuthAssets.invalidate(); } catch (err) { logging.error(new errors.InternalServerError({ message: tpl(messages.activateFailed, {theme: loadedTheme.name}), diff --git a/ghost/core/core/frontend/src/admin-auth/index.html b/ghost/core/core/frontend/public/admin-auth/index.html similarity index 100% rename from ghost/core/core/frontend/src/admin-auth/index.html rename to ghost/core/core/frontend/public/admin-auth/index.html diff --git a/ghost/core/core/frontend/src/admin-auth/message-handler.js b/ghost/core/core/frontend/public/admin-auth/message-handler.js similarity index 100% rename from ghost/core/core/frontend/src/admin-auth/message-handler.js rename to ghost/core/core/frontend/public/admin-auth/message-handler.js diff --git a/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js b/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js deleted file mode 100644 index df9c52e5fa..0000000000 --- a/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js +++ /dev/null @@ -1,68 +0,0 @@ -// const debug = require('@tryghost/debug')('comments-counts-assets'); -const Minifier = require('@tryghost/minifier'); -const path = require('path'); -const fs = require('fs'); -const logging = require('@tryghost/logging'); -const config = require('../../../shared/config'); -const urlUtils = require('../../../shared/url-utils'); -const AssetsMinificationBase = require('./AssetsMinificationBase'); - -module.exports = class AdminAuthAssets extends AssetsMinificationBase { - constructor(options = {}) { - super(options); - - this.src = options.src || path.join(config.get('paths').assetSrc, 'admin-auth'); - /** @private */ - this.dest = options.dest || path.join(config.getContentPath('public'), 'admin-auth'); - - this.minifier = new Minifier({src: this.src, dest: this.dest}); - - try { - // TODO: don't do this synchronously - fs.mkdirSync(this.dest, {recursive: true}); - fs.copyFileSync(path.join(this.src, 'index.html'), path.join(this.dest, 'index.html')); - } catch (error) { - if (error.code === 'EACCES') { - logging.error('Ghost was not able to write admin-auth asset files due to permissions.'); - return; - } - - throw error; - } - } - - /** - * @override - */ - generateGlobs() { - return { - 'admin-auth.min.js': '*.js' - }; - } - - /** - * @private - */ - generateReplacements() { - // Clean the URL, only keep schema, host and port (without trailing slashes or subdirectory) - const url = new URL(urlUtils.getSiteUrl()); - const origin = url.origin; - - return { - // Properly encode the origin - '\'{{SITE_ORIGIN}}\'': JSON.stringify(origin) - }; - } - - /** - * Minify, move into the destination directory, and clear existing asset files. - * - * @override - * @returns {Promise} - */ - async load() { - const globs = this.generateGlobs(); - const replacements = this.generateReplacements(); - await this.minify(globs, {replacements}); - } -}; diff --git a/ghost/core/core/frontend/services/assets-minification/index.js b/ghost/core/core/frontend/services/assets-minification/index.js index 3a9e37f7a2..0af4c9c115 100644 --- a/ghost/core/core/frontend/services/assets-minification/index.js +++ b/ghost/core/core/frontend/services/assets-minification/index.js @@ -1,10 +1,7 @@ -const AdminAuthAssets = require('./AdminAuthAssets'); const CardAssets = require('./CardAssets'); -const adminAuthAssets = new AdminAuthAssets(); const cardAssets = new CardAssets(); module.exports = { - adminAuthAssets, cardAssets }; diff --git a/ghost/core/core/server/web/admin/app.js b/ghost/core/core/server/web/admin/app.js index 090e3eb16d..faf63bc3e3 100644 --- a/ghost/core/core/server/web/admin/app.js +++ b/ghost/core/core/server/web/admin/app.js @@ -9,7 +9,7 @@ const shared = require('../shared'); const errorHandler = require('@tryghost/mw-error-handler'); const sentry = require('../../../shared/sentry'); const redirectAdminUrls = require('./middleware/redirect-admin-urls'); -const bridge = require('../../../bridge'); +const createServeAuthFrameFileMw = require('./middleware/serve-auth-frame-file'); /** * @@ -39,7 +39,7 @@ module.exports = function setupAdminApp() { // request to the Admin API /users/me/ endpoint to check if the user is logged in. // // Used by comments-ui to add moderation options to front-end comments when logged in. - adminApp.use('/auth-frame', bridge.ensureAdminAuthAssetsMiddleware(), function authFrameMw(req, res, next) { + adminApp.use('/auth-frame', function authFrameMw(req, res, next) { // only render content when we have an Admin session cookie, // otherwise return a 204 to avoid JS and API requests being made unnecessarily try { @@ -52,9 +52,7 @@ module.exports = function setupAdminApp() { } catch (err) { next(err); } - }, serveStatic( - path.join(config.getContentPath('public'), 'admin-auth') - )); + }, createServeAuthFrameFileMw(config, urlUtils)); // Ember CLI's live-reload script if (config.get('env') === 'development') { diff --git a/ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js b/ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js new file mode 100644 index 0000000000..f08b6bec81 --- /dev/null +++ b/ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js @@ -0,0 +1,35 @@ +const path = require('node:path'); +const fs = require('node:fs/promises'); + +function createServeAuthFrameFileMw(config, urlUtils) { + const placeholders = { + '{{SITE_ORIGIN}}': new URL(urlUtils.getSiteUrl()).origin + }; + + return function serveAuthFrameFileMw(req, res, next) { + const filename = path.parse(req.url).base; + + let basePath = path.join(config.get('paths').publicFilePath, 'admin-auth'); + let filePath; + + if (filename === '') { + filePath = path.join(basePath, 'index.html'); + } else { + filePath = path.join(basePath, filename); + } + + return fs.readFile(filePath).then((data) => { + let dataString = data.toString(); + + for (const [key, value] of Object.entries(placeholders)) { + dataString = dataString.replace(key, value); + } + + res.end(dataString); + }).catch(() => { + return next(); + }); + }; +} + +module.exports = createServeAuthFrameFileMw; diff --git a/ghost/core/package.json b/ghost/core/package.json index af46f1f0f6..67abf79978 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -23,7 +23,7 @@ "archive": "npm pack", "dev": "node --watch index.js", "build:assets:css": "postcss core/frontend/public/ghost.css --no-map --use cssnano -o core/frontend/public/ghost.min.css", - "build:assets:js": "../minifier/bin/minify core/frontend/public/comment-counts.js core/frontend/public/comment-counts.min.js && ../minifier/bin/minify core/frontend/public/member-attribution.js core/frontend/public/member-attribution.min.js", + "build:assets:js": "../minifier/bin/minify core/frontend/public/comment-counts.js core/frontend/public/comment-counts.min.js && ../minifier/bin/minify core/frontend/public/member-attribution.js core/frontend/public/member-attribution.min.js && ../minifier/bin/minify core/frontend/public/admin-auth/message-handler.js core/frontend/public/admin-auth/admin-auth.min.js", "build:assets": "yarn build:assets:css && yarn build:assets:js", "test": "yarn test:unit", "test:base": "mocha --reporter dot --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js", diff --git a/ghost/core/test/unit/server/web/admin/middleware.test.js b/ghost/core/test/unit/server/web/admin/middleware.test.js index 00dc587845..95457a7848 100644 --- a/ghost/core/test/unit/server/web/admin/middleware.test.js +++ b/ghost/core/test/unit/server/web/admin/middleware.test.js @@ -1,8 +1,11 @@ +const fs = require('node:fs/promises'); const should = require('should'); const sinon = require('sinon'); // Thing we are testing const redirectAdminUrls = require('../../../../../core/server/web/admin/middleware/redirect-admin-urls'); +const createServeAuthFrameFileMw = require('../../../../../core/server/web/admin/middleware/serve-auth-frame-file'); +const path = require('node:path'); describe('Admin App', function () { afterEach(function () { @@ -60,5 +63,144 @@ describe('Admin App', function () { res.redirect.called.should.be.false(); }); }); + + describe('serveAuthFrameFile', function () { + let config; + let urlUtils; + let readFile; + + const siteUrl = 'https://foo.bar'; + const publicFilePath = 'foo/bar/public'; + + const indexContent = '

Hello, world!

'; + const fooJsContent = '(function() { console.log("Hello, world!"); })();'; + const fooJsContentWithSiteOrigin = '(function() { console.log("{{SITE_ORIGIN}}"); })();'; + + function createMiddleware() { + return createServeAuthFrameFileMw(config, urlUtils); + } + + beforeEach(function () { + config = { + get: sinon.stub() + }; + + config.get.withArgs('paths').returns({ + publicFilePath + }); + + urlUtils = { + getSiteUrl: sinon.stub().returns(siteUrl) + }; + readFile = sinon.stub(fs, 'readFile'); + + const adminAuthFilePath = filename => path.join(publicFilePath, 'admin-auth', filename); + + readFile.withArgs(adminAuthFilePath('index.html')) + .resolves(Buffer.from(indexContent)); + readFile.withArgs(adminAuthFilePath('foo.js')) + .resolves(Buffer.from(fooJsContent)); + readFile.withArgs(adminAuthFilePath('foo-2.js')) + .resolves(Buffer.from(fooJsContentWithSiteOrigin)); + readFile.withArgs(adminAuthFilePath('bar.js')) + .rejects(new Error('File not found')); + }); + + afterEach(function () { + readFile.restore(); + }); + + it('should serve index.html if the url is /', async function () { + const middleware = createMiddleware(); + + const req = { + url: '/' + }; + const res = { + end: sinon.stub() + }; + const next = sinon.stub(); + + await middleware(req, res, next); + + res.end.calledWith(indexContent).should.be.true(); + next.called.should.be.false(); + }); + + it('should serve the correct file corresponding to the url', async function () { + const middleware = createMiddleware(); + + const req = { + url: '/foo.js' + }; + const res = { + end: sinon.stub() + }; + const next = sinon.stub(); + + await middleware(req, res, next); + + res.end.calledWith(fooJsContent).should.be.true(); + next.called.should.be.false(); + }); + + it('should replace {{SITE_ORIGIN}} with the site url', async function () { + const middleware = createMiddleware(); + + const req = { + url: '/foo-2.js' + }; + const res = { + end: sinon.stub() + }; + const next = sinon.stub(); + + await middleware(req, res, next); + + res.end.calledOnce.should.be.true(); + + const args = res.end.getCall(0).args; + args[0].toString().includes(siteUrl).should.be.true(); + args[0].toString().includes(`{{SITE_ORIGIN}}`).should.be.false(); + }); + + it('should not allow path traversal', async function () { + const middleware = createMiddleware(); + + const req = { + url: '/foo/../../foo.js' + }; + const res = { + end: sinon.stub() + }; + const next = sinon.stub(); + + await middleware(req, res, next); + + res.end.calledOnce.should.be.true(); + + // Because we use base name when resolving the file, foo.js should be served + res.end.calledWith(fooJsContent).should.be.true(); + + next.calledOnce.should.be.false(); + }); + + it('should call next if the file is not found', async function () { + const middleware = createMiddleware(); + + const req = { + url: '/bar.js' + }; + const res = { + end: sinon.stub() + }; + const next = sinon.stub(); + + await middleware(req, res, next); + + res.end.calledOnce.should.be.false(); + next.calledOnce.should.be.true(); + }); + }); }); });