From b008f0e5ed315dd6b1d2f52ab391b82e105ef8f4 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 12:21:41 -0500 Subject: [PATCH 1/7] Cleaned up TOTP code (tests, TS, etc) (#30187) no ref *I recommend reviewing this change one commit at a time.* This change: - Adds unit tests for TOTP helpers - Converts TOTP generator to TypeScript - Renames "OTP" to "TOTP" (to disambiguate with "HOTP") - Avoids overwriting global options This should have no user impact. --- ghost/core/core/server/services/auth/otp.js | 35 ---------------- .../services/auth/session/session-service.js | 6 +-- ghost/core/core/server/services/auth/totp.ts | 30 ++++++++++++++ .../unit/server/services/auth/totp.test.ts | 40 +++++++++++++++++++ 4 files changed, 73 insertions(+), 38 deletions(-) delete mode 100644 ghost/core/core/server/services/auth/otp.js create mode 100644 ghost/core/core/server/services/auth/totp.ts create mode 100644 ghost/core/test/unit/server/services/auth/totp.test.ts diff --git a/ghost/core/core/server/services/auth/otp.js b/ghost/core/core/server/services/auth/otp.js deleted file mode 100644 index 076bb879e31..00000000000 --- a/ghost/core/core/server/services/auth/otp.js +++ /dev/null @@ -1,35 +0,0 @@ -const { totp } = require('otplib'); - -totp.options = { - digits: 6, - step: 60, - window: [10, 10], -}; - -/** - * Generate a TOTP token for a user - * @param {string} userId - The user's ID - * @param {string} secret - The admin session secret - * @param {string} [context] - Optional session-specific context to bind the token - * @returns {string} - The generated 6-digit token - */ -function generate(userId, secret, context = '') { - return totp.generate(`${secret}${userId}${context}`); -} - -/** - * Verify a TOTP token for a user - * @param {string} userId - The user's ID - * @param {string} token - The token to verify - * @param {string} secret - The admin session secret - * @param {string} [context] - Optional session-specific context to bind the token - * @returns {boolean} - Whether the token is valid - */ -function verify(userId, token, secret, context = '') { - return totp.check(token, `${secret}${userId}${context}`); -} - -module.exports = { - generate, - verify, -}; diff --git a/ghost/core/core/server/services/auth/session/session-service.js b/ghost/core/core/server/services/auth/session/session-service.js index ea707c87f15..1fd48011a41 100644 --- a/ghost/core/core/server/services/auth/session/session-service.js +++ b/ghost/core/core/server/services/auth/session/session-service.js @@ -4,7 +4,7 @@ const crypto = require('crypto'); const emailTemplate = require('./emails/signin'); const UAParser = require('ua-parser-js'); const got = require('got').default; -const otp = require('../otp'); +const totp = require('../totp'); const IPV4_REGEX = /^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/; const IPV6_REGEX = /^(?:[A-F0-9]{1,4}:){7}[A-F0-9]{1,4}$/i; @@ -108,7 +108,7 @@ module.exports = function createSessionService({ return false; } - const verified = otp.verify(session.user_id, token, secret, session.auth_code_challenge); + const verified = totp.verify(session.user_id, token, secret, session.auth_code_challenge); if (verified) { invalidateAuthCodeChallenge(session); @@ -319,7 +319,7 @@ module.exports = function createSessionService({ const session = await getSession(req, res); const secret = getSettingsCache('admin_session_secret'); const challenge = ensureAuthCodeChallenge(session); - return otp.generate(session.user_id, secret, challenge); + return totp.generate(session.user_id, secret, challenge); } /** diff --git a/ghost/core/core/server/services/auth/totp.ts b/ghost/core/core/server/services/auth/totp.ts new file mode 100644 index 00000000000..943b5892a31 --- /dev/null +++ b/ghost/core/core/server/services/auth/totp.ts @@ -0,0 +1,30 @@ +import * as otplib from 'otplib'; + +const totp = otplib.totp.clone({ + digits: 6, + step: 60, + window: [10, 10], +}); + +/** + * Generate a TOTP token for a user + * @param userId The user's ID + * @param secret The admin session secret + * @param [context] Optional session-specific context to bind the token + * @returns The generated 6-digit token + */ +export function generate(userId: string, secret: string, context = ''): string { + return totp.generate(`${secret}${userId}${context}`); +} + +/** + * Verify a TOTP token for a user + * @param userId The user's ID + * @param token The token to verify + * @param secret The admin session secret + * @param [context] Optional session-specific context to bind the token + * @returns Whether the token is valid + */ +export function verify(userId: string, token: string, secret: string, context = ''): boolean { + return totp.check(token, `${secret}${userId}${context}`); +} diff --git a/ghost/core/test/unit/server/services/auth/totp.test.ts b/ghost/core/test/unit/server/services/auth/totp.test.ts new file mode 100644 index 00000000000..d31bdb5d596 --- /dev/null +++ b/ghost/core/test/unit/server/services/auth/totp.test.ts @@ -0,0 +1,40 @@ +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +import { generate, verify } from '../../../../../core/server/services/auth/totp'; + +describe('TOTP service', function () { + let clock: sinon.SinonFakeTimers; + + beforeEach(function () { + clock = sinon.useFakeTimers(new Date('2026-01-01T12:00:00.000Z')); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('generates a six-digit token that verifies for same user and secret', function () { + const token = generate('user-1', 'admin-session-secret'); + + assert.match(token, /^\d{6}$/); + assert.equal(verify('user-1', token, 'admin-session-secret'), true); + }); + + it('binds tokens to user, secret, and session context', function () { + const token = generate('user-1', 'admin-session-secret', 'session-1'); + + assert.equal(verify('user-2', token, 'admin-session-secret', 'session-1'), false); + assert.equal(verify('user-1', token, 'other-session-secret', 'session-1'), false); + assert.equal(verify('user-1', token, 'admin-session-secret', 'session-2'), false); + }); + + it('accepts tokens for configured ten-minute window', function () { + const token = generate('user-1', 'admin-session-secret'); + + clock.tick(10 * 60 * 1000); + assert.equal(verify('user-1', token, 'admin-session-secret'), true); + + clock.tick(60 * 1000); + assert.equal(verify('user-1', token, 'admin-session-secret'), false); + }); +}); From 1242ef4e77a4d1bdde9f7508cdf931039a3724da Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 16:17:37 -0500 Subject: [PATCH 2/7] Replaced promise package usage with simple `await`s and loops (#30190) no ref This should have no user impact. The `sequence` export from `@tryghost/promise` can be replaced with simple `await`s and loops. This brings us closer to removing the package. --- .../core/server/data/exporter/exporter.js | 10 +- .../data/importer/importers/data/base.js | 5 +- .../data/custom-theme-settings-importer.js | 5 +- .../importer/importers/data/data-importer.js | 11 ++- .../importers/data/settings-importer.js | 5 +- .../importer/importers/data/tags-importer.js | 5 +- .../data/migrations/init/1-create-tables.js | 15 ++- .../data/schema/fixtures/fixture-manager.js | 99 +++++++++---------- .../core/core/server/models/base/listeners.js | 67 ++++++------- ghost/core/core/server/models/post.js | 7 +- .../core/server/models/relations/authors.js | 17 +++- 11 files changed, 128 insertions(+), 118 deletions(-) diff --git a/ghost/core/core/server/data/exporter/exporter.js b/ghost/core/core/server/data/exporter/exporter.js index 5d319b353fb..0fb4defc78a 100644 --- a/ghost/core/core/server/data/exporter/exporter.js +++ b/ghost/core/core/server/data/exporter/exporter.js @@ -4,7 +4,6 @@ const commands = require('../schema').commands; const ghostVersion = require('@tryghost/version'); const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); -const { sequence } = require('@tryghost/promise'); const messages = { errorExportingData: 'Error exporting data', @@ -38,11 +37,10 @@ const doExport = async function doExport(options) { try { const tables = await commands.getTables(options.transacting); - const tableData = await sequence( - tables.map((tableName) => async () => { - return exportTable(tableName, options); - }), - ); + const tableData = []; + for (const tableName of tables) { + tableData.push(await exportTable(tableName, options)); + } const exportData = { meta: { diff --git a/ghost/core/core/server/data/importer/importers/data/base.js b/ghost/core/core/server/data/importer/importers/data/base.js index 29ed420514b..e54f80038a2 100644 --- a/ghost/core/core/server/data/importer/importers/data/base.js +++ b/ghost/core/core/server/data/importer/importers/data/base.js @@ -2,7 +2,6 @@ const debug = require('@tryghost/debug')('importer:base'); const _ = require('lodash'); const ObjectId = require('bson-objectid').default; const errors = require('@tryghost/errors'); -const { sequence } = require('@tryghost/promise'); const models = require('../../../../models'); class Base { @@ -346,7 +345,9 @@ class Base { }); }); - await sequence(ops); + for (const op of ops) { + await op(); + } } } diff --git a/ghost/core/core/server/data/importer/importers/data/custom-theme-settings-importer.js b/ghost/core/core/server/data/importer/importers/data/custom-theme-settings-importer.js index e25d8468595..dc9f3eb603a 100644 --- a/ghost/core/core/server/data/importer/importers/data/custom-theme-settings-importer.js +++ b/ghost/core/core/server/data/importer/importers/data/custom-theme-settings-importer.js @@ -3,7 +3,6 @@ const debug = require('@tryghost/debug')('importer:roles'); const BaseImporter = require('./base'); const models = require('../../../../models'); const { activate } = require('../../../../services/themes/activate'); -const { sequence } = require('@tryghost/promise'); class CustomThemeSettingsImporter extends BaseImporter { constructor(allDataFromFile) { @@ -55,7 +54,9 @@ class CustomThemeSettingsImporter extends BaseImporter { }); }); - await sequence(ops); + for (const op of ops) { + await op(); + } const theme = await models.Settings.findOne({ key: 'active_theme' }, options); const currentTheme = theme && theme.get('value'); diff --git a/ghost/core/core/server/data/importer/importers/data/data-importer.js b/ghost/core/core/server/data/importer/importers/data/data-importer.js index 9b42a5ad618..b29df8c2a89 100644 --- a/ghost/core/core/server/data/importer/importers/data/data-importer.js +++ b/ghost/core/core/server/data/importer/importers/data/data-importer.js @@ -3,7 +3,6 @@ const ObjectId = require('bson-objectid').default; const semver = require('semver'); const { IncorrectUsageError, DataImportError } = require('@tryghost/errors'); const debug = require('@tryghost/debug')('importer:data'); -const { sequence } = require('@tryghost/promise'); const models = require('../../../../models'); const PostsImporter = require('./posts-importer'); const TagsImporter = require('./tags-importer'); @@ -168,7 +167,7 @@ DataImporter = { * - already exist in the db * so we only need to map imported products */ - ops.push(() => { + ops.push(async () => { const importedStripePrices = importers.stripe_prices.importedData; const importedProducts = importers.products.importedData; const productOps = []; @@ -189,10 +188,14 @@ DataImporter = { }); }); - return sequence(productOps); + for (const productOp of productOps) { + await productOp(); + } }); - await sequence(ops); + for (const op of ops) { + await op(); + } // Errors preventing import: if (errors.length > 0) { diff --git a/ghost/core/core/server/data/importer/importers/data/settings-importer.js b/ghost/core/core/server/data/importer/importers/data/settings-importer.js index af9d2295e1b..38de7c8b8f5 100644 --- a/ghost/core/core/server/data/importer/importers/data/settings-importer.js +++ b/ghost/core/core/server/data/importer/importers/data/settings-importer.js @@ -7,7 +7,6 @@ const defaultSettings = require('../../../schema').defaultSettings; const keyGroupMapper = require('../../../../api/endpoints/utils/serializers/input/utils/settings-key-group-mapper'); const keyTypeMapper = require('../../../../api/endpoints/utils/serializers/input/utils/settings-key-type-mapper'); const { WRITABLE_KEYS_ALLOWLIST } = require('../../../../../shared/labs'); -const { sequence } = require('@tryghost/promise'); const labsDefaults = JSON.parse(defaultSettings.labs.labs.defaultValue); const ignoredSettings = [ @@ -281,7 +280,9 @@ class SettingsImporter extends BaseImporter { }); }); - await sequence(ops); + for (const op of ops) { + await op(); + } } } diff --git a/ghost/core/core/server/data/importer/importers/data/tags-importer.js b/ghost/core/core/server/data/importer/importers/data/tags-importer.js index 4af5c3661f7..e82abdfcb74 100644 --- a/ghost/core/core/server/data/importer/importers/data/tags-importer.js +++ b/ghost/core/core/server/data/importer/importers/data/tags-importer.js @@ -2,7 +2,6 @@ const debug = require('@tryghost/debug')('importer:tags'); const _ = require('lodash'); const BaseImporter = require('./base'); const models = require('../../../../models'); -const { sequence } = require('@tryghost/promise'); class TagsImporter extends BaseImporter { constructor(allDataFromFile) { @@ -71,7 +70,9 @@ class TagsImporter extends BaseImporter { }); }); - await sequence(ops); + for (const op of ops) { + await op(); + } } } diff --git a/ghost/core/core/server/data/migrations/init/1-create-tables.js b/ghost/core/core/server/data/migrations/init/1-create-tables.js index 218aba3615b..2ae6f5e40e1 100644 --- a/ghost/core/core/server/data/migrations/init/1-create-tables.js +++ b/ghost/core/core/server/data/migrations/init/1-create-tables.js @@ -3,7 +3,6 @@ const schema = require('../../schema').tables; const views = require('../../schema').views; const logging = require('@tryghost/logging'); const schemaTables = Object.keys(schema); -const { sequence } = require('@tryghost/promise'); module.exports.up = async (options) => { const connection = options.connection; @@ -11,12 +10,10 @@ module.exports.up = async (options) => { const existingTables = await commands.getTables(connection); const missingTables = schemaTables.filter((t) => !existingTables.includes(t)); - await sequence( - missingTables.map((table) => async () => { - logging.info('Creating table: ' + table); - await commands.createTable(table, connection); - }), - ); + for (const table of missingTables) { + logging.info('Creating table: ' + table); + await commands.createTable(table, connection); + } // Create views after tables exist. View creation is idempotent // (createViewOrReplace) so adding a new view to views.js does not require @@ -38,9 +35,9 @@ module.exports.up = async (options) => { // Reference between tables! schemaTables.reverse(); - await sequence(schemaTables.map(table => async () => { + for (const table of schemaTables) { logging.info('Drop table: ' + table); await commands.deleteTable(table, connection); - })); + } }; */ diff --git a/ghost/core/core/server/data/schema/fixtures/fixture-manager.js b/ghost/core/core/server/data/schema/fixtures/fixture-manager.js index 960cf673dc2..26653398800 100644 --- a/ghost/core/core/server/data/schema/fixtures/fixture-manager.js +++ b/ghost/core/core/server/data/schema/fixtures/fixture-manager.js @@ -1,6 +1,5 @@ const _ = require('lodash'); const logging = require('@tryghost/logging'); -const { sequence } = require('@tryghost/promise'); const models = require('../../../models'); const baseUtils = require('../../../models/base/utils'); @@ -125,23 +124,15 @@ class FixtureManager { const userRolesRelation = this.fixtures.relations.find((r) => r.from.relation === 'roles'); await this.addFixturesForRelation(userRolesRelation, localOptions); - await sequence( - this.fixtures.models - .filter((m) => !['User', 'Role'].includes(m.name)) - .map((model) => () => { - logging.info('Model: ' + model.name); - return this.addFixturesForModel(model, localOptions); - }), - ); + for (const model of this.fixtures.models.filter((m) => !['User', 'Role'].includes(m.name))) { + logging.info('Model: ' + model.name); + await this.addFixturesForModel(model, localOptions); + } - await sequence( - this.fixtures.relations - .filter((r) => r.from.relation !== 'roles') - .map((relation) => () => { - logging.info('Relation: ' + relation.from.model + ' to ' + relation.to.model); - return this.addFixturesForRelation(relation, localOptions); - }), - ); + for (const relation of this.fixtures.relations.filter((r) => r.from.relation !== 'roles')) { + logging.info('Relation: ' + relation.from.model + ' to ' + relation.to.model); + await this.addFixturesForRelation(relation, localOptions); + } } /* @@ -358,29 +349,28 @@ class FixtureManager { }); } - const results = await sequence( - modelFixture.entries.map((entry) => async () => { - let data = {}; - - // CASE: if id is specified, only query by id - if (entry.id) { - data.id = entry.id; - } else if (entry.slug) { - data.slug = entry.slug; - } else { - data = _.cloneDeep(entry); - } + const results = []; + for (const entry of modelFixture.entries) { + let data = {}; + + // CASE: if id is specified, only query by id + if (entry.id) { + data.id = entry.id; + } else if (entry.slug) { + data.slug = entry.slug; + } else { + data = _.cloneDeep(entry); + } - if (modelFixture.name === 'Post') { - data.status = 'all'; - } + if (modelFixture.name === 'Post') { + data.status = 'all'; + } - const found = await models[modelFixture.name].findOne(data, options); - if (!found) { - return models[modelFixture.name].add(entry, options); - } - }), - ); + const found = await models[modelFixture.name].findOne(data, options); + if (!found) { + results.push(await models[modelFixture.name].add(entry, options)); + } + } return { expected: modelFixture.entries.length, done: _.compact(results).length }; } @@ -440,22 +430,25 @@ class FixtureManager { }); }); - const result = await sequence(ops); + const result = []; + for (const op of ops) { + result.push(await op()); + } return { expected: max, done: _(result).map('length').sum() }; } async removeFixturesForModel(modelFixture, options) { - const results = await sequence( - modelFixture.entries.map((entry) => async () => { - const found = models[modelFixture.name].findOne( - entry.id ? { id: entry.id } : entry, - options, - ); - if (found) { - return models[modelFixture.name].destroy(_.extend(options, { id: found.id })); - } - }), - ); + const results = []; + for (const entry of modelFixture.entries) { + const found = await models[modelFixture.name].findOne( + entry.id ? { id: entry.id } : entry, + options, + ); + const result = found + ? await models[modelFixture.name].destroy(_.extend(options, { id: found.id })) + : undefined; + results.push(result); + } return { expected: modelFixture.entries.length, done: results.length }; } @@ -486,7 +479,11 @@ class FixtureManager { }); }); - return await sequence(ops); + const results = []; + for (const op of ops) { + results.push(await op()); + } + return results; } } diff --git a/ghost/core/core/server/models/base/listeners.js b/ghost/core/core/server/models/base/listeners.js index 5c4713f5920..a195e55c01b 100644 --- a/ghost/core/core/server/models/base/listeners.js +++ b/ghost/core/core/server/models/base/listeners.js @@ -3,7 +3,6 @@ const _ = require('lodash'); const models = require('../../models'); const logging = require('@tryghost/logging'); const errors = require('@tryghost/errors'); -const { sequence } = require('@tryghost/promise'); // Listen to settings.timezone.edited and settings.notifications.edited to bind extra logic to settings, similar to the bridge and member service const events = require('../../lib/common/events'); @@ -42,41 +41,39 @@ const onTimezoneEdited = function (settingModel, options) { return; } - await sequence( - results.map((post) => async () => { - const newPublishedAtMoment = moment(post.get('published_at')).add( - timezoneOffsetDiff, - 'minutes', + for (const post of results) { + const newPublishedAtMoment = moment(post.get('published_at')).add( + timezoneOffsetDiff, + 'minutes', + ); + + /** + * CASE: + * - your configured TZ is GMT+01:00 + * - now is 10AM +01:00 (9AM UTC) + * - your post should be published 8PM +01:00 (7PM UTC) + * - you reconfigure your blog TZ to GMT+08:00 + * - now is 5PM +08:00 (9AM UTC) + * - if we don't change the published_at, 7PM + 8 hours === next day 5AM + * - so we update published_at to 7PM - 480minutes === 11AM UTC + * - 11AM UTC === 7PM +08:00 + */ + if (newPublishedAtMoment.isBefore(moment().add(5, 'minutes'))) { + post.set('status', 'draft'); + } else { + post.set('published_at', newPublishedAtMoment.toDate()); + } + + try { + await models.Post.edit(post.toJSON(), _.merge({ id: post.id }, options)); + } catch (err) { + logging.error( + new errors.InternalServerError({ + err, + }), ); - - /** - * CASE: - * - your configured TZ is GMT+01:00 - * - now is 10AM +01:00 (9AM UTC) - * - your post should be published 8PM +01:00 (7PM UTC) - * - you reconfigure your blog TZ to GMT+08:00 - * - now is 5PM +08:00 (9AM UTC) - * - if we don't change the published_at, 7PM + 8 hours === next day 5AM - * - so we update published_at to 7PM - 480minutes === 11AM UTC - * - 11AM UTC === 7PM +08:00 - */ - if (newPublishedAtMoment.isBefore(moment().add(5, 'minutes'))) { - post.set('status', 'draft'); - } else { - post.set('published_at', newPublishedAtMoment.toDate()); - } - - try { - await models.Post.edit(post.toJSON(), _.merge({ id: post.id }, options)); - } catch (err) { - logging.error( - new errors.InternalServerError({ - err, - }), - ); - } - }), - ); + } + } } catch (err) { logging.error( new errors.InternalServerError({ diff --git a/ghost/core/core/server/models/post.js b/ghost/core/core/server/models/post.js index f5c888933d1..2bd66c350c0 100644 --- a/ghost/core/core/server/models/post.js +++ b/ghost/core/core/server/models/post.js @@ -2,7 +2,6 @@ const _ = require('lodash'); const crypto = require('crypto'); const moment = require('moment'); -const { sequence } = require('@tryghost/promise'); const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); const nql = require('@tryghost/nql'); @@ -1052,7 +1051,11 @@ Post = ghostBookshelf.Model.extend( } } - return sequence(ops); + const results = []; + for (const op of ops) { + results.push(await op()); + } + return results; }, published_by: function publishedBy() { diff --git a/ghost/core/core/server/models/relations/authors.js b/ghost/core/core/server/models/relations/authors.js index bc1a455c672..f997af71a0e 100644 --- a/ghost/core/core/server/models/relations/authors.js +++ b/ghost/core/core/server/models/relations/authors.js @@ -1,7 +1,6 @@ const _ = require('lodash'); const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); -const { sequence } = require('@tryghost/promise'); const { setIsRoles } = require('../role-utils'); const messages = { @@ -130,7 +129,13 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) { return proto.onSaving.call(this, model, attrs, options); }); - return sequence(ops); + return (async () => { + const results = []; + for (const op of ops) { + results.push(await op()); + } + return results; + })(); }, serialize: function serialize(options) { @@ -224,7 +229,13 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) { }); }); - return sequence(ops); + return (async () => { + const results = []; + for (const op of ops) { + results.push(await op()); + } + return results; + })(); }, }, { From 77b3bba5738641b99f8283d8e645e8234a934a56 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 16:18:01 -0500 Subject: [PATCH 3/7] Inlined un-called bulk deletion method (#30191) no ref `bulkDestroyWhere` was exported from a Bookshelf plugin, but only called in once place. Let's simply inline it. `git grep bulkDestroyWhere` returns no results after this change. --- .../models/base/plugins/bulk-operations.js | 29 +++++-------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/ghost/core/core/server/models/base/plugins/bulk-operations.js b/ghost/core/core/server/models/base/plugins/bulk-operations.js index c25f26d8996..7706032e2da 100644 --- a/ghost/core/core/server/models/base/plugins/bulk-operations.js +++ b/ghost/core/core/server/models/base/plugins/bulk-operations.js @@ -111,23 +111,6 @@ module.exports = function (Bookshelf) { ); }, - /** - * Delete rows matching a where strategy (e.g. byNQL, byIds, byColumnValues). - * Pure data operation — no action logging. - * - * @param {object} options - * @param {Iterable<(qb: import('knex').QueryBuilder) => void>} options.where - Where strategy - * @param {object} [options.transacting] - Knex transaction - * @param {string} [options.tableName] - Table to delete from (defaults to model's table) - * @returns {Promise} Total affected rows - */ - bulkDestroyWhere: async function bulkDestroyWhere({ where, transacting, tableName }) { - tableName = tableName || this.prototype.tableName; - return bulkWhereOperation(Bookshelf.knex, tableName, { where, transacting }, (qb) => - qb.del(), - ); - }, - /** * Edit rows by ID list, with action logging. * @@ -189,11 +172,15 @@ module.exports = function (Bookshelf) { } try { - await this.bulkDestroyWhere({ - where: byColumnValues(options.column ?? 'id', ids), - transacting: options.transacting, + await bulkWhereOperation( + Bookshelf.knex, tableName, - }); + { + where: byColumnValues(options.column ?? 'id', ids), + transacting: options.transacting, + }, + (qb) => qb.del(), + ); return { successful: ids.length, unsuccessful: 0, errors: [], unsuccessfulData: [] }; } catch (err) { From 1933583f96b59afb8cb9c4b8f1d18b81fb90cc23 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 16:18:14 -0500 Subject: [PATCH 4/7] TypeScriptified "session from token" middleware (#30192) no ref --- .../server/services/auth/session/index.js | 2 +- .../auth/session/session-from-token.js | 69 ------------------- .../auth/session/session-from-token.ts | 49 +++++++++++++ ...ken.test.js => session-from-token.test.ts} | 10 +-- 4 files changed, 55 insertions(+), 75 deletions(-) delete mode 100644 ghost/core/core/server/services/auth/session/session-from-token.js create mode 100644 ghost/core/core/server/services/auth/session/session-from-token.ts rename ghost/core/test/unit/server/services/auth/{session-from-token.test.js => session-from-token.test.ts} (83%) diff --git a/ghost/core/core/server/services/auth/session/index.js b/ghost/core/core/server/services/auth/session/index.js index 00cf5e4446f..a77c44ccc16 100644 --- a/ghost/core/core/server/services/auth/session/index.js +++ b/ghost/core/core/server/services/auth/session/index.js @@ -1,6 +1,6 @@ const adapterManager = require('../../adapter-manager').default; const createSessionService = require('./session-service'); -const sessionFromToken = require('./session-from-token'); +const { sessionFromToken } = require('./session-from-token'); const createSessionMiddleware = require('./middleware'); const settingsCache = require('../../../../shared/settings-cache'); const { GhostMailer } = require('../../mail'); diff --git a/ghost/core/core/server/services/auth/session/session-from-token.js b/ghost/core/core/server/services/auth/session/session-from-token.js deleted file mode 100644 index 2e893f50182..00000000000 --- a/ghost/core/core/server/services/auth/session/session-from-token.js +++ /dev/null @@ -1,69 +0,0 @@ -module.exports = SessionFromToken; - -/** - * @typedef {object} User - * @prop {string} id - */ - -/** - * @typedef {import('express').Request} Req - * @typedef {import('express').Response} Res - * @typedef {import('express').NextFunction} Next - * @typedef {import('express').RequestHandler} RequestHandler - */ - -/** - * Returns a connect middleware function which exchanges a token for a session - * - * @template Token - * @template Lookup - * - * @param { object } deps - * @param { (req: Req) => Promise } deps.getTokenFromRequest - * @param { (token: Token) => Promise } deps.getLookupFromToken - * @param { (lookup: Lookup) => Promise } deps.findUserByLookup - * @param { (req: Req, res: Res, user: User) => Promise } deps.createSession - * @param { boolean } deps.callNextWithError - Whether next should be call with an error or just pass through - * - * @returns {RequestHandler} - */ -function SessionFromToken({ - getTokenFromRequest, - getLookupFromToken, - findUserByLookup, - createSession, - callNextWithError, -}) { - /** - * @param {Req} req - * @param {Res} res - * @param {Next} next - * @returns {Promise} - */ - async function handler(req, res, next) { - try { - const token = await getTokenFromRequest(req); - if (!token) { - return next(); - } - const email = await getLookupFromToken(token); - if (!email) { - return next(); - } - const user = await findUserByLookup(email); - if (!user) { - return next(); - } - await createSession(req, res, user); - next(); - } catch (err) { - if (callNextWithError) { - next(err); - } else { - next(); - } - } - } - - return handler; -} diff --git a/ghost/core/core/server/services/auth/session/session-from-token.ts b/ghost/core/core/server/services/auth/session/session-from-token.ts new file mode 100644 index 00000000000..fa86d64ba67 --- /dev/null +++ b/ghost/core/core/server/services/auth/session/session-from-token.ts @@ -0,0 +1,49 @@ +import type * as express from 'express'; + +type User = { id: string }; + +/** + * Returns a connect middleware function which exchanges a token for a session + */ +export const sessionFromToken = + ({ + getTokenFromRequest, + getLookupFromToken, + findUserByLookup, + createSession, + callNextWithError, + }: Readonly<{ + getTokenFromRequest: (req: express.Request) => Promise; + getLookupFromToken: (token: Token) => Promise; + findUserByLookup: (lookup: Lookup) => Promise; + createSession: (req: express.Request, res: express.Response, user: User) => Promise; + callNextWithError: boolean; + }>): (( + req: express.Request, + res: express.Response, + next: express.NextFunction, + ) => Promise) => + async (req, res, next) => { + try { + const token = await getTokenFromRequest(req); + if (!token) { + return next(); + } + const email = await getLookupFromToken(token); + if (!email) { + return next(); + } + const user = await findUserByLookup(email); + if (!user) { + return next(); + } + await createSession(req, res, user); + next(); + } catch (err) { + if (callNextWithError) { + next(err); + } else { + next(); + } + } + }; diff --git a/ghost/core/test/unit/server/services/auth/session-from-token.test.js b/ghost/core/test/unit/server/services/auth/session-from-token.test.ts similarity index 83% rename from ghost/core/test/unit/server/services/auth/session-from-token.test.js rename to ghost/core/test/unit/server/services/auth/session-from-token.test.ts index 60d67a1e981..64b09baf77a 100644 --- a/ghost/core/test/unit/server/services/auth/session-from-token.test.js +++ b/ghost/core/test/unit/server/services/auth/session-from-token.test.ts @@ -1,8 +1,8 @@ -const express = require('express'); -const sinon = require('sinon'); -const SessionFromToken = require('../../../../../core/server/services/auth/session/session-from-token'); +import express from 'express'; +import sinon from 'sinon'; +import { sessionFromToken } from '../../../../../core/server/services/auth/session/session-from-token'; -describe('SessionFromToken', function () { +describe('sessionFromToken', function () { it('Parses the request, matches the user to the token, sets the user on req.user and calls createSession', async function () { const createSession = sinon.spy(async (req, res, user) => { req.session = user; @@ -11,7 +11,7 @@ describe('SessionFromToken', function () { const getTokenFromRequest = sinon.spy(async (req) => req.token); const getLookupFromToken = sinon.spy(async (token) => token.email); - const handler = SessionFromToken({ + const handler = sessionFromToken({ getTokenFromRequest, getLookupFromToken, findUserByLookup, From 106bf69d3e866d66176fc62c0cf5649632a163db Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 16:18:31 -0500 Subject: [PATCH 5/7] TypeScriptified API endpoint utils test (#30193) no ref --- .../unit/api/canary/utils/{index.test.js => index.test.ts} | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) rename ghost/core/test/unit/api/canary/utils/{index.test.js => index.test.ts} (75%) diff --git a/ghost/core/test/unit/api/canary/utils/index.test.js b/ghost/core/test/unit/api/canary/utils/index.test.ts similarity index 75% rename from ghost/core/test/unit/api/canary/utils/index.test.js rename to ghost/core/test/unit/api/canary/utils/index.test.ts index 243d0d96324..e9d0d24e687 100644 --- a/ghost/core/test/unit/api/canary/utils/index.test.js +++ b/ghost/core/test/unit/api/canary/utils/index.test.ts @@ -1,6 +1,7 @@ -const assert = require('node:assert/strict'); -const sinon = require('sinon'); -const utils = require('../../../../../core/server/api/endpoints/utils'); +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +// @ts-expect-error This module lacks type definitions. +import utils from '../../../../../core/server/api/endpoints/utils'; describe('Unit: endpoints/utils/index', function () { afterEach(function () { From 5f9b42416ff0859746f8b866447ffb608ffcd4c6 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 16:18:46 -0500 Subject: [PATCH 6/7] TypeScriptified E2E API clock helper (#30194) no ref --- ghost/core/test/utils/{clock-utils.js => clock-utils.ts} | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) rename ghost/core/test/utils/{clock-utils.js => clock-utils.ts} (72%) diff --git a/ghost/core/test/utils/clock-utils.js b/ghost/core/test/utils/clock-utils.ts similarity index 72% rename from ghost/core/test/utils/clock-utils.js rename to ghost/core/test/utils/clock-utils.ts index 8a0a799c9b8..c6a372a31db 100644 --- a/ghost/core/test/utils/clock-utils.js +++ b/ghost/core/test/utils/clock-utils.ts @@ -1,4 +1,4 @@ -const sinon = require('sinon'); +import * as sinon from 'sinon'; /** * Pin the system clock to `now` for deterministic time-based assertions while @@ -10,12 +10,7 @@ const sinon = require('sinon'); * * Returns the sinon clock — `clock.tick()`/`clock.tickAsync()` advance time; * `clock.restore()` (or `sinon.restore()`) undoes it. - * - * @param {Date|number} [now] initial time (defaults to the real current time) - * @returns {import('sinon').SinonFakeTimers} */ -function mockSystemTime(now = Date.now()) { +export function mockSystemTime(now = Date.now()) { return sinon.useFakeTimers({ now, toFake: ['Date'] }); } - -module.exports = { mockSystemTime }; From 81292b004cf59591f03d7dbe01f28f31c09ee813 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 21 Aug 2026 17:14:26 -0500 Subject: [PATCH 7/7] TypeScriptified `uncapitalise` middleware test (#30196) no ref This also rewrites it in a more realistic way, instead of mocking `req` and `res`. --- .../shared/middleware/uncapitalise.test.js | 243 ------------------ .../shared/middleware/uncapitalise.test.ts | 136 ++++++++++ 2 files changed, 136 insertions(+), 243 deletions(-) delete mode 100644 ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.js create mode 100644 ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.ts diff --git a/ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.js b/ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.js deleted file mode 100644 index 01e964b8f2b..00000000000 --- a/ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.js +++ /dev/null @@ -1,243 +0,0 @@ -const sinon = require('sinon'); -const uncapitalise = require('../../../../../../core/server/web/shared/middleware/uncapitalise'); - -// NOTE: all urls will have had trailing slashes added before uncapitalise is called - -describe('Middleware: uncapitalise', function () { - let res; - let req; - let next; - - beforeEach(function () { - res = { - redirect: sinon.spy(), - set: sinon.spy(), - }; - req = {}; - next = sinon.spy(); - }); - - afterEach(function () { - sinon.restore(); - }); - - describe('Signup or reset request', function () { - it('[signup] does nothing if there are no capitals in req.path', function () { - req.path = '/ghost/signup/'; - uncapitalise(req, res, next); - - sinon.assert.calledOnce(next); - }); - - it('[signup] does nothing if there are no capitals in the baseUrl', function () { - req.baseUrl = '/ghost/signup/'; - req.path = ''; - uncapitalise(req, res, next); - - sinon.assert.calledOnce(next); - }); - - it('[signup] does nothing if there are no capitals except in a token', function () { - req.baseUrl = '/blog'; - req.path = '/ghost/signup/XEB123'; - - uncapitalise(req, res, next); - - sinon.assert.calledOnce(next); - }); - - it('[reset] does nothing if there are no capitals except in a token', function () { - req.baseUrl = '/blog'; - req.path = - '/ghost/reset/NCR3NjY4NzI1ODI1OHzlcmlzZHNAZ51haWwuY29tfEpWeGxRWHUzZ3Y0cEpQRkNYYzQvbUZyc2xFSVozU3lIZHZWeFJLRml6cY54'; - uncapitalise(req, res, next); - - sinon.assert.calledOnce(next); - }); - - it('[signup] redirects if there are capitals in req.path', function () { - req.path = '/ghost/SignUP/'; - req.url = req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith(res.redirect, 301, '/ghost/signup/'); - }); - - it('[signup] redirects if there are capitals in req.baseUrl', function () { - req.baseUrl = '/ghost/SignUP/'; - req.path = ''; - req.url = req.path; - req.originalUrl = req.baseUrl + req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith(res.redirect, 301, '/ghost/signup/'); - }); - - it('[signup] redirects correctly if there are capitals in req.path and req.baseUrl', function () { - req.baseUrl = '/Blog'; - req.path = '/ghosT/signUp/'; - req.url = req.path; - req.originalUrl = req.baseUrl + req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith(res.redirect, 301, '/blog/ghost/signup/'); - }); - - it('[signup] redirects correctly with capitals in req.path if there is a token', function () { - req.path = '/ghosT/sigNup/XEB123'; - req.url = req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith(res.redirect, 301, '/ghost/signup/XEB123'); - }); - - it('[reset] redirects correctly with capitals in req.path & req.baseUrl if there is a token', function () { - req.baseUrl = '/Blog'; - req.path = - '/Ghost/Reset/NCR3NjY4NzI1ODI1OHzlcmlzZHNAZ51haWwuY29tfEpWeGxRWHUzZ3Y0cEpQRkNYYzQvbUZyc2xFSVozU3lIZHZWeFJLRml6cY54'; - req.url = req.path; - req.originalUrl = req.baseUrl + req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith( - res.redirect, - 301, - '/blog/ghost/reset/NCR3NjY4NzI1ODI1OHzlcmlzZHNAZ51haWwuY29tfEpWeGxRWHUzZ3Y0cEpQRkNYYzQvbUZyc2xFSVozU3lIZHZWeFJLRml6cY54', - ); - }); - }); - - describe('An API request', function () { - ['v0.1', 'canary', 'v10', null].forEach((apiVersion) => { - const getApiPath = (version) => { - return version ? `/${version}` : ''; - }; - - describe(`for ${apiVersion}`, function () { - it('does nothing if there are no capitals', function () { - req.path = `/ghost/api${getApiPath(apiVersion)}/endpoint/`; - uncapitalise(req, res, next); - - sinon.assert.calledOnce(next); - }); - - it('version identifier is uppercase', function () { - // CASE: capitalizing "empty" string does not make sense - if (apiVersion === null) { - return; - } - - req.path = `/ghost/api${getApiPath(apiVersion).toUpperCase()}/endpoint/`; - req.url = req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith( - res.redirect, - 301, - `/ghost/api${getApiPath(apiVersion)}/endpoint/`, - ); - }); - - it('redirects to the lower case slug if there are capitals', function () { - req.path = `/ghost/api${getApiPath(apiVersion)}/ASDfJ/`; - req.url = req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith(res.redirect, 301, `/ghost/api${getApiPath(apiVersion)}/asdfj/`); - }); - - it('redirects to the lower case slug if there are capitals in req.baseUrl', function () { - req.baseUrl = '/Blog'; - req.path = `/ghost/api${getApiPath(apiVersion)}/ASDfJ/`; - req.url = req.path; - req.originalUrl = req.baseUrl + req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith( - res.redirect, - 301, - `/blog/ghost/api${getApiPath(apiVersion)}/asdfj/`, - ); - }); - - it('does not convert any capitals after the endpoint', function () { - const query = '?filter=mAgic'; - req.path = `/Ghost/API${getApiPath(apiVersion)}/settings/is_private/`; - req.url = `${req.path}${query}`; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith( - res.redirect, - 301, - `/ghost/api${getApiPath(apiVersion)}/settings/is_private/${query}`, - ); - }); - - it('does not convert any capitals after the endpoint with baseUrl', function () { - const query = '?filter=mAgic'; - req.baseUrl = '/Blog'; - req.path = `/ghost/api${getApiPath(apiVersion)}/mail/test@example.COM/`; - req.url = `${req.path}${query}`; - req.originalUrl = `${req.baseUrl}${req.path}${query}`; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith( - res.redirect, - 301, - `/blog/ghost/api${getApiPath(apiVersion)}/mail/test@example.COM/${query}`, - ); - }); - }); - }); - }); - - describe('Any other request', function () { - it('does nothing if there are no capitals', function () { - req.path = '/this-is-my-blog-post'; - uncapitalise(req, res, next); - - sinon.assert.calledOnce(next); - }); - - it('redirects to the lower case slug if there are capitals', function () { - req.path = '/THis-iS-my-BLOg-poSt'; - req.url = req.path; - - uncapitalise(req, res, next); - - sinon.assert.notCalled(next); - sinon.assert.calledOnce(res.redirect); - sinon.assert.calledWith(res.redirect, 301, '/this-is-my-blog-post'); - }); - }); -}); diff --git a/ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.ts b/ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.ts new file mode 100644 index 00000000000..3f115b71ec4 --- /dev/null +++ b/ghost/core/test/unit/server/web/shared/middleware/uncapitalise.test.ts @@ -0,0 +1,136 @@ +import express, { type Express } from 'express'; +import request from 'supertest'; +// @ts-expect-error This module lacks type definitions. +import uncapitalise from '../../../../../../core/server/web/shared/middleware/uncapitalise.js'; + +// NOTE: all URLs have trailing slashes before uncapitalise runs + +describe('Middleware: uncapitalise', function () { + function createApp(baseUrl?: string): Express { + const app = express(); + if (baseUrl) { + app.use(baseUrl, uncapitalise); + } else { + app.use(uncapitalise); + } + app.use((_req, res) => res.sendStatus(204)); + return app; + } + + async function expectNoRedirect(path: string, baseUrl?: string) { + await request(createApp(baseUrl)) + .get(`${baseUrl || ''}${path}`) + .expect(204); + } + + async function expectRedirect(path: string, location: string, baseUrl?: string) { + await request(createApp(baseUrl)) + .get(`${baseUrl || ''}${path}`) + .expect(301) + .expect('Location', location); + } + + describe('Signup or reset request', function () { + it('[signup] does nothing if there are no capitals in req.path', async function () { + await expectNoRedirect('/ghost/signup/'); + }); + + it('[signup] does nothing if there are no capitals in baseUrl', async function () { + await expectNoRedirect('/', '/ghost/signup'); + }); + + it('[signup] does nothing if there are no capitals except in a token', async function () { + await expectNoRedirect('/ghost/signup/XEB123', '/blog'); + }); + + it('[reset] does nothing if there are no capitals except in a token', async function () { + await expectNoRedirect( + '/ghost/reset/NCR3NjY4NzI1ODI1OHzlcmlzZHNAZ51haWwuY29tfEpWeGxRWHUzZ3Y0cEpQRkNYYzQvbUZyc2xFSVozU3lIZHZWeFJLRml6cY54', + '/blog', + ); + }); + + it('[signup] redirects if there are capitals in req.path', async function () { + await expectRedirect('/ghost/SignUP/', '/ghost/signup/'); + }); + + it('[signup] redirects if there are capitals in req.baseUrl', async function () { + await expectRedirect('/', '/ghost/signup/', '/ghost/SignUP'); + }); + + it('[signup] redirects correctly if there are capitals in req.path and req.baseUrl', async function () { + await expectRedirect('/ghosT/signUp/', '/blog/ghost/signup/', '/Blog'); + }); + + it('[signup] redirects correctly with capitals in req.path if there is a token', async function () { + await expectRedirect('/ghosT/sigNup/XEB123', '/ghost/signup/XEB123'); + }); + + it('[reset] redirects correctly with capitals in req.path & req.baseUrl if there is a token', async function () { + const token = + 'NCR3NjY4NzI1ODI1OHzlcmlzZHNAZ51haWwuY29tfEpWeGxRWHUzZ3Y0cEpQRkNYYzQvbUZyc2xFSVozU3lIZHZWeFJLRml6cY54'; + + await expectRedirect(`/Ghost/Reset/${token}`, `/blog/ghost/reset/${token}`, '/Blog'); + }); + }); + + describe('An API request', function () { + ['v0.1', 'canary', 'v10', null].forEach((apiVersion) => { + const apiPath = apiVersion ? `/${apiVersion}` : ''; + + describe(`for ${apiVersion}`, function () { + it('does nothing if there are no capitals', async function () { + await expectNoRedirect(`/ghost/api${apiPath}/endpoint/`); + }); + + it('version identifier is uppercase', async function () { + if (apiVersion === null) { + return; + } + + await expectRedirect( + `/ghost/api${apiPath.toUpperCase()}/endpoint/`, + `/ghost/api${apiPath}/endpoint/`, + ); + }); + + it('redirects to lower-case slug if there are capitals', async function () { + await expectRedirect(`/ghost/api${apiPath}/ASDfJ/`, `/ghost/api${apiPath}/asdfj/`); + }); + + it('redirects to lower-case slug if there are capitals in req.baseUrl', async function () { + await expectRedirect( + `/ghost/api${apiPath}/ASDfJ/`, + `/blog/ghost/api${apiPath}/asdfj/`, + '/Blog', + ); + }); + + it('does not convert capitals after endpoint', async function () { + await expectRedirect( + `/Ghost/API${apiPath}/settings/is_private/?filter=mAgic`, + `/ghost/api${apiPath}/settings/is_private/?filter=mAgic`, + ); + }); + + it('does not convert capitals after endpoint with baseUrl', async function () { + await expectRedirect( + `/ghost/api${apiPath}/mail/test@example.COM/?filter=mAgic`, + `/blog/ghost/api${apiPath}/mail/test@example.COM/?filter=mAgic`, + '/Blog', + ); + }); + }); + }); + }); + + describe('Any other request', function () { + it('does nothing if there are no capitals', async function () { + await expectNoRedirect('/this-is-my-blog-post'); + }); + + it('redirects to lower-case slug if there are capitals', async function () { + await expectRedirect('/THis-iS-my-BLOg-poSt', '/this-is-my-blog-post'); + }); + }); +});