From 66f78af6a193f6c3ddf0366ccd89124ebf289139 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Tue, 3 Oct 2017 13:40:53 +0200 Subject: [PATCH] Refactored private blogging app: use settings cache (#9086) no issue - preparation for #9001 - no need to require the settings API, we can simply fetch the data from the settings cache - the settings API uses the settings cache anyway --- .../apps/private-blogging/lib/middleware.js | 127 +++++++-------- .../private-blogging/tests/middleware_spec.js | 144 ++++++------------ 2 files changed, 101 insertions(+), 170 deletions(-) diff --git a/core/server/apps/private-blogging/lib/middleware.js b/core/server/apps/private-blogging/lib/middleware.js index 1db8032db2..5632ab7c5e 100644 --- a/core/server/apps/private-blogging/lib/middleware.js +++ b/core/server/apps/private-blogging/lib/middleware.js @@ -1,47 +1,39 @@ -var _ = require('lodash'), - fs = require('fs'), - session = require('cookie-session'), - crypto = require('crypto'), - path = require('path'), - Promise = require('bluebird'), - config = require('../../../config'), - api = require('../../../api'), - utils = require('../../../utils'), - i18n = require('../../../i18n'), +var fs = require('fs'), + session = require('cookie-session'), + crypto = require('crypto'), + path = require('path'), + config = require('../../../config'), + utils = require('../../../utils'), + i18n = require('../../../i18n'), + settingsCache = require('../../../settings/cache'), privateRoute = '/' + config.get('routeKeywords').private + '/', privateBlogging; function verifySessionHash(salt, hash) { if (!salt || !hash) { - return Promise.resolve(false); + return false; } - return api.settings.read({context: {internal: true}, key: 'password'}).then(function then(response) { - var hasher = crypto.createHash('sha256'); - - hasher.update(response.settings[0].value + salt, 'utf8'); - - return hasher.digest('hex') === hash; - }); + var hasher = crypto.createHash('sha256'); + hasher.update(settingsCache.get('password') + salt, 'utf8'); + return hasher.digest('hex') === hash; } privateBlogging = { checkIsPrivate: function checkIsPrivate(req, res, next) { - return api.settings.read({context: {internal: true}, key: 'is_private'}).then(function then(response) { - var pass = response.settings[0]; + var isPrivateBlog = settingsCache.get('is_private'); - if (_.isEmpty(pass.value) || pass.value === 'false') { - res.isPrivateBlog = false; - return next(); - } + if (!isPrivateBlog) { + res.isPrivateBlog = false; + return next(); + } - res.isPrivateBlog = true; + res.isPrivateBlog = true; - return session({ - maxAge: utils.ONE_MONTH_MS, - signed: false - })(req, res, next); - }); + return session({ + maxAge: utils.ONE_MONTH_MS, + signed: false + })(req, res, next); }, filterPrivateRoutes: function filterPrivateRoutes(req, res, next) { @@ -50,7 +42,7 @@ privateBlogging = { } if (req.url.lastIndexOf('/robots.txt', 0) === 0) { - fs.readFile(path.resolve(__dirname, '../', 'robots.txt'), function readFile(err, buf) { + return fs.readFile(path.resolve(__dirname, '../', 'robots.txt'), function readFile(err, buf) { if (err) { return next(err); } @@ -63,25 +55,24 @@ privateBlogging = { res.end(buf); }); - } else { - return privateBlogging.authenticatePrivateSession(req, res, next); } + + privateBlogging.authenticatePrivateSession(req, res, next); }, authenticatePrivateSession: function authenticatePrivateSession(req, res, next) { var hash = req.session.token || '', salt = req.session.salt || '', + isVerified = verifySessionHash(salt, hash), url; - return verifySessionHash(salt, hash).then(function then(isVerified) { - if (isVerified) { - return next(); - } else { - url = utils.url.urlFor({relativeUrl: privateRoute}); - url += req.url === '/' ? '' : '?r=' + encodeURIComponent(req.url); - return res.redirect(url); - } - }); + if (isVerified) { + return next(); + } else { + url = utils.url.urlFor({relativeUrl: privateRoute}); + url += req.url === '/' ? '' : '?r=' + encodeURIComponent(req.url); + return res.redirect(url); + } }, // This is here so a call to /private/ after a session is verified will redirect to home; @@ -91,16 +82,15 @@ privateBlogging = { } var hash = req.session.token || '', - salt = req.session.salt || ''; + salt = req.session.salt || '', + isVerified = verifySessionHash(salt, hash); - return verifySessionHash(salt, hash).then(function then(isVerified) { - if (isVerified) { - // redirect to home if user is already authenticated - return res.redirect(utils.url.urlFor('home', true)); - } else { - return next(); - } - }); + if (isVerified) { + // redirect to home if user is already authenticated + return res.redirect(utils.url.urlFor('home', true)); + } else { + return next(); + } }, authenticateProtection: function authenticateProtection(req, res, next) { @@ -109,27 +99,24 @@ privateBlogging = { return next(); } - var bodyPass = req.body.password; + var bodyPass = req.body.password, + pass = settingsCache.get('password'), + hasher = crypto.createHash('sha256'), + salt = Date.now().toString(), + forward = req.query && req.query.r ? req.query.r : '/'; - return api.settings.read({context: {internal: true}, key: 'password'}).then(function then(response) { - var pass = response.settings[0], - hasher = crypto.createHash('sha256'), - salt = Date.now().toString(), - forward = req.query && req.query.r ? req.query.r : '/'; + if (pass === bodyPass) { + hasher.update(bodyPass + salt, 'utf8'); + req.session.token = hasher.digest('hex'); + req.session.salt = salt; - if (pass.value === bodyPass) { - hasher.update(bodyPass + salt, 'utf8'); - req.session.token = hasher.digest('hex'); - req.session.salt = salt; - - return res.redirect(utils.url.urlFor({relativeUrl: decodeURIComponent(forward)})); - } else { - res.error = { - message: i18n.t('errors.middleware.privateblogging.wrongPassword') - }; - return next(); - } - }); + return res.redirect(utils.url.urlFor({relativeUrl: decodeURIComponent(forward)})); + } else { + res.error = { + message: i18n.t('errors.middleware.privateblogging.wrongPassword') + }; + return next(); + } } }; diff --git a/core/server/apps/private-blogging/tests/middleware_spec.js b/core/server/apps/private-blogging/tests/middleware_spec.js index 260fd395d0..e4cc4bffc9 100644 --- a/core/server/apps/private-blogging/tests/middleware_spec.js +++ b/core/server/apps/private-blogging/tests/middleware_spec.js @@ -2,24 +2,19 @@ var should = require('should'), // jshint ignore:line sinon = require('sinon'), crypto = require('crypto'), - Promise = require('bluebird'), - api = require('../../../api'), + settingsCache = require('../../../settings/cache'), fs = require('fs'), - privateBlogging = require('../lib/middleware'), - sandbox = sinon.sandbox.create(); function hash(password, salt) { var hasher = crypto.createHash('sha256'); - hasher.update(password + salt, 'utf8'); - return hasher.digest('hex'); } describe('Private Blogging', function () { - var apiSettingsStub; + var settingsStub; afterEach(function () { sandbox.restore(); @@ -31,39 +26,23 @@ describe('Private Blogging', function () { beforeEach(function () { req = {}; res = {}; - apiSettingsStub = sandbox.stub(api.settings, 'read'); + settingsStub = sandbox.stub(settingsCache, 'get'); next = sandbox.spy(); }); - it('checkIsPrivate should call next if not private', function (done) { - apiSettingsStub.withArgs(sinon.match.has('key', 'is_private')).returns(Promise.resolve({ - settings: [{ - key: 'is_private', - value: 'false' - }] - })); + it('checkIsPrivate should call next if not private', function () { + settingsStub.withArgs('is_private').returns(false); - privateBlogging.checkIsPrivate(req, res, next).then(function () { - next.called.should.be.true(); - res.isPrivateBlog.should.be.false(); - - done(); - }).catch(done); + privateBlogging.checkIsPrivate(req, res, next); + next.called.should.be.true(); + res.isPrivateBlog.should.be.false(); }); - it('checkIsPrivate should load session if private', function (done) { - apiSettingsStub.withArgs(sinon.match.has('key', 'is_private')).returns(Promise.resolve({ - settings: [{ - key: 'is_private', - value: 'true' - }] - })); + it('checkIsPrivate should load session if private', function () { + settingsStub.withArgs('is_private').returns(true); - privateBlogging.checkIsPrivate(req, res, next).then(function () { - res.isPrivateBlog.should.be.true(); - - done(); - }).catch(done); + privateBlogging.checkIsPrivate(req, res, next); + res.isPrivateBlog.should.be.true(); }); describe('not private', function () { @@ -118,56 +97,44 @@ describe('Private Blogging', function () { it('filterPrivateRoutes: sitemap redirects to /private', function () { req.path = req.url = '/sitemap.xml'; - return privateBlogging.filterPrivateRoutes(req, res, next) - .then(function () { - res.redirect.calledOnce.should.be.true(); - }); + privateBlogging.filterPrivateRoutes(req, res, next); + res.redirect.calledOnce.should.be.true(); }); it('filterPrivateRoutes: sitemap with params redirects to /private', function () { req.url = '/sitemap.xml?weird=param'; req.path = '/sitemap.xml'; - return privateBlogging.filterPrivateRoutes(req, res, next) - .then(function () { - res.redirect.calledOnce.should.be.true(); - }); + privateBlogging.filterPrivateRoutes(req, res, next); + res.redirect.calledOnce.should.be.true(); }); it('filterPrivateRoutes: rss redirects to /private', function () { req.path = req.url = '/rss/'; - return privateBlogging.filterPrivateRoutes(req, res, next) - .then(function () { - res.redirect.calledOnce.should.be.true(); - }); + privateBlogging.filterPrivateRoutes(req, res, next); + res.redirect.calledOnce.should.be.true(); }); it('filterPrivateRoutes: author rss redirects to /private', function () { req.path = req.url = '/author/halfdan/rss/'; - return privateBlogging.filterPrivateRoutes(req, res, next) - .then(function () { - res.redirect.calledOnce.should.be.true(); - }); + privateBlogging.filterPrivateRoutes(req, res, next); + res.redirect.calledOnce.should.be.true(); }); it('filterPrivateRoutes: tag rss redirects to /private', function () { req.path = req.url = '/tag/slimer/rss/'; - return privateBlogging.filterPrivateRoutes(req, res, next) - .then(function () { - res.redirect.calledOnce.should.be.true(); - }); + privateBlogging.filterPrivateRoutes(req, res, next); + res.redirect.calledOnce.should.be.true(); }); it('filterPrivateRoutes: rss plus something redirects to /private', function () { req.path = req.url = '/rss/sometag'; - return privateBlogging.filterPrivateRoutes(req, res, next) - .then(function () { - res.redirect.calledOnce.should.be.true(); - }); + privateBlogging.filterPrivateRoutes(req, res, next); + res.redirect.calledOnce.should.be.true(); }); it('filterPrivateRoutes should render custom robots.txt', function () { @@ -190,15 +157,10 @@ describe('Private Blogging', function () { describe('with hash verification', function () { beforeEach(function () { - apiSettingsStub.withArgs(sinon.match.has('key', 'password')).returns(Promise.resolve({ - settings: [{ - key: 'password', - value: 'rightpassword' - }] - })); + settingsStub.withArgs('password').returns('rightpassword'); }); - it('authenticatePrivateSession should return next if hash is verified', function (done) { + it('authenticatePrivateSession should return next if hash is verified', function () { var salt = Date.now().toString(); req.session = { @@ -206,14 +168,11 @@ describe('Private Blogging', function () { salt: salt }; - privateBlogging.authenticatePrivateSession(req, res, next).then(function () { - next.called.should.be.true(); - - done(); - }).catch(done); + privateBlogging.authenticatePrivateSession(req, res, next); + next.called.should.be.true(); }); - it('authenticatePrivateSession should redirect if hash is not verified', function (done) { + it('authenticatePrivateSession should redirect if hash is not verified', function () { req.url = '/welcome'; req.session = { token: 'wrongpassword', @@ -221,14 +180,11 @@ describe('Private Blogging', function () { }; res.redirect = sandbox.spy(); - privateBlogging.authenticatePrivateSession(req, res, next).then(function () { - res.redirect.called.should.be.true(); - - done(); - }).catch(done); + privateBlogging.authenticatePrivateSession(req, res, next); + res.redirect.called.should.be.true(); }); - it('isPrivateSessionAuth should redirect if hash is verified', function (done) { + it('isPrivateSessionAuth should redirect if hash is verified', function () { var salt = Date.now().toString(); req.session = { @@ -237,47 +193,35 @@ describe('Private Blogging', function () { }; res.redirect = sandbox.spy(); - privateBlogging.isPrivateSessionAuth(req, res, next).then(function () { - res.redirect.called.should.be.true(); - - done(); - }).catch(done); + privateBlogging.isPrivateSessionAuth(req, res, next); + res.redirect.called.should.be.true(); }); - it('isPrivateSessionAuth should return next if hash is not verified', function (done) { + it('isPrivateSessionAuth should return next if hash is not verified', function () { req.session = { token: 'wrongpassword', salt: Date.now().toString() }; - privateBlogging.isPrivateSessionAuth(req, res, next).then(function () { - next.called.should.be.true(); - - done(); - }).catch(done); + privateBlogging.isPrivateSessionAuth(req, res, next); + next.called.should.be.true(); }); - it('authenticateProtection should return next if password is incorrect', function (done) { + it('authenticateProtection should return next if password is incorrect', function () { req.body = {password: 'wrongpassword'}; - privateBlogging.authenticateProtection(req, res, next).then(function () { - res.error.should.not.be.empty(); - next.called.should.be.true(); - - done(); - }).catch(done); + privateBlogging.authenticateProtection(req, res, next); + res.error.should.not.be.empty(); + next.called.should.be.true(); }); - it('authenticateProtection should redirect if password is correct', function (done) { + it('authenticateProtection should redirect if password is correct', function () { req.body = {password: 'rightpassword'}; req.session = {}; res.redirect = sandbox.spy(); - privateBlogging.authenticateProtection(req, res, next).then(function () { - res.redirect.called.should.be.true(); - - done(); - }).catch(done); + privateBlogging.authenticateProtection(req, res, next); + res.redirect.called.should.be.true(); }); }); });