From 7fdddf34b37402641b64e2e9d9a3aec0ac55fda0 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Mon, 18 Jan 2021 17:03:41 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Added=20multiple=20use=20grace?= =?UTF-8?q?=20period=20to=20tokens=20(#12519)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes https://github.com/TryGhost/Ghost/issues/12347 This change allows a token to be used multiple times for the first 10 seconds after its initial use, this will stop dynamic link checking software from invaliding magic links. --- core/server/models/single-use-token.js | 23 +++++++---- .../models/model_single_use_token_spec.js | 30 -------------- test/unit/models/single-use-token_spec.js | 41 +++++++++++++++++++ 3 files changed, 55 insertions(+), 39 deletions(-) delete mode 100644 test/regression/models/model_single_use_token_spec.js create mode 100644 test/unit/models/single-use-token_spec.js diff --git a/core/server/models/single-use-token.js b/core/server/models/single-use-token.js index a4a9f2f911..550e59cd7d 100644 --- a/core/server/models/single-use-token.js +++ b/core/server/models/single-use-token.js @@ -1,5 +1,6 @@ const ghostBookshelf = require('./base'); const crypto = require('crypto'); +const logging = require('../../shared/logging'); const SingleUseToken = ghostBookshelf.Model.extend({ tableName: 'tokens', @@ -16,19 +17,23 @@ const SingleUseToken = ghostBookshelf.Model.extend({ } }, { async findOne(data, unfilteredOptions = {}) { - if (!unfilteredOptions.transacting) { - return ghostBookshelf.transaction((transacting) => { - return this.findOne(data, Object.assign({transacting}, unfilteredOptions)); - }); - } const model = await ghostBookshelf.Model.findOne.call(this, data, unfilteredOptions); if (model) { - await this.destroy(Object.assign({ - destroyBy: { - id: model.id + setTimeout(async () => { + try { + await this.destroy(Object.assign({ + destroyBy: { + id: model.id + } + }, { + ...unfilteredOptions, + transacting: null + })); + } catch (err) { + logging.error(err); } - }, unfilteredOptions)); + }, 10000); } return model; diff --git a/test/regression/models/model_single_use_token_spec.js b/test/regression/models/model_single_use_token_spec.js deleted file mode 100644 index f79ad950c0..0000000000 --- a/test/regression/models/model_single_use_token_spec.js +++ /dev/null @@ -1,30 +0,0 @@ -const models = require('../../../core/server/models'); -const should = require('should'); - -describe('Regression: models/single-use-token', function () { - before(function () { - models.init(); - }); - - describe('findOne', function () { - it('Does not allow the same token to be read twice', async function () { - const insertedToken = await models.SingleUseToken.add({ - data: 'some_data' - }, {}); - - const tokenFirstRead = await models.SingleUseToken.findOne({ - token: insertedToken.get('token') - }); - - should.exist(tokenFirstRead); - should.equal(tokenFirstRead.id, insertedToken.id); - - const tokenSecondRead = await models.SingleUseToken.findOne({ - token: insertedToken.get('token') - }); - - should.not.exist(tokenSecondRead); - }); - }); -}); - diff --git a/test/unit/models/single-use-token_spec.js b/test/unit/models/single-use-token_spec.js new file mode 100644 index 0000000000..6b01c1131f --- /dev/null +++ b/test/unit/models/single-use-token_spec.js @@ -0,0 +1,41 @@ +const models = require('../../../core/server/models'); +const should = require('should'); +const sinon = require('sinon'); + +let clock; +let sandbox; + +describe('Unit: models/single-use-token', function () { + before(function () { + models.init(); + sandbox = sinon.createSandbox(); + clock = sandbox.useFakeTimers(); + }); + + after(function () { + clock.restore(); + sandbox.restore(); + }); + + describe('fn: findOne', function () { + it('Calls destroy after the grace period', async function () { + const data = {}; + const options = {}; + const fakeModel = { + id: 'fake_id' + }; + + const findOneSuperStub = sandbox.stub(models.Base.Model, 'findOne').resolves(fakeModel); + const destroyStub = sandbox.stub(models.SingleUseToken, 'destroy').resolves(); + + await models.SingleUseToken.findOne(data, options); + + should.ok(findOneSuperStub.calledWith(data, options), 'super.findOne was called'); + + clock.tick(10000); + + should.ok(destroyStub.called, 'destroy was called after 10 seconds'); + }); + }); +}); +