Merge pull request #5232 from jaswilli/pp-fixup

Change payload storage in session cookie

core/server/middleware/middleware.js

@@ -5,9 +5,9 @@
 var _           = require('lodash'),
     fs          = require('fs'),
     express     = require('express'),
-    bcrypt      = require('bcryptjs'),
     busboy      = require('./ghost-busboy'),
     config      = require('../config'),
+    crypto      = require('crypto'),
     path        = require('path'),
     api         = require('../api'),
     passport    = require('passport'),
@@ -81,14 +81,17 @@ function sslForbiddenOrRedirect(opt) {
     return response;
 }
 
-function verifySessionHash(hash) {
+function verifySessionHash(salt, hash) {
-    if (!hash) {
+    if (!salt || !hash) {
         return Promise.resolve(false);
     }
-    var bcryptCompare = Promise.promisify(bcrypt.compare);
+
     return api.settings.read({context: {internal: true}, key: 'password'}).then(function (response) {
-        var pass = response.settings[0].value;
+        var hasher = crypto.createHash('sha256');
-        return bcryptCompare(pass, hash);
+
+        hasher.update(response.settings[0].value + salt, 'utf8');
+
+        return hasher.digest('hex') === hash;
     });
 }
 
@@ -344,14 +347,17 @@ middleware = {
     checkIsPrivate: function (req, res, next) {
         return api.settings.read({context: {internal: true}, key: 'isPrivate'}).then(function (response) {
             var pass = response.settings[0];
+
             if (_.isEmpty(pass.value) || pass.value === 'false') {
                 res.isPrivateBlog = false;
                 return next();
             }
+
             res.isPrivateBlog = true;
+
             return session({
                 maxAge: utils.ONE_MONTH_MS,
-                keys: ['isPrivateBlog']
+                signed: false
             })(req, res, next);
         });
     },
@@ -360,7 +366,9 @@ middleware = {
         if (res.isAdmin || !res.isPrivateBlog || req.url.lastIndexOf('/private/', 0) === 0) {
             return next();
         }
-        if (req.url.lastIndexOf('/rss', 0) === 0 || req.url.lastIndexOf('/sitemap', 0) === 0) { // take care of rss and sitemap 404's
+
+        // take care of rss and sitemap 404s
+        if (req.url.lastIndexOf('/rss', 0) === 0 || req.url.lastIndexOf('/sitemap', 0) === 0) {
             return errors.error404(req, res, next);
         } else if (req.url.lastIndexOf('/robots.txt', 0) === 0) {
             fs.readFile(path.join(config.paths.corePath, 'shared', 'private-robots.txt'), function (err, buf) {
@@ -380,8 +388,10 @@ middleware = {
     },
 
     authenticatePrivateSession: function (req, res, next) {
-        var clientHash = req.session.token || '';
+        var hash = req.session.token || '',
-        return verifySessionHash(clientHash).then(function (isVerified) {
+            salt = req.session.salt || '';
+
+        return verifySessionHash(salt, hash).then(function (isVerified) {
             if (isVerified) {
                 return next();
             } else {
@@ -395,10 +405,14 @@ middleware = {
         if (!res.isPrivateBlog) {
             return res.redirect(config.urlFor('home', true));
         }
-        var hash = req.session.token || '';
+
-        return verifySessionHash(hash).then(function (isVerified) {
+        var hash = req.session.token || '',
+            salt = req.session.salt || '';
+
+        return verifySessionHash(salt, hash).then(function (isVerified) {
             if (isVerified) {
-                return res.redirect(config.urlFor('home', true)); // redirect to home if user is already authenticated
+                // redirect to home if user is already authenticated
+                return res.redirect(config.urlFor('home', true));
             } else {
                 return next();
             }
@@ -446,18 +460,24 @@ middleware = {
     },
 
     authenticateProtection: function (req, res, next) {
-        if (res.error) { // if errors have been generated from the previous call
+        // if errors have been generated from the previous call
+        if (res.error) {
             return next();
         }
-        var bodyPass = req.body.password,
+
-            bcryptHash = Promise.promisify(bcrypt.hash);
+        var bodyPass = req.body.password;
+
         return api.settings.read({context: {internal: true}, key: 'password'}).then(function (response) {
-            var pass = response.settings[0];
+            var pass = response.settings[0],
+                hasher = crypto.createHash('sha256'),
+                salt = Date.now().toString();
+
             if (pass.value === bodyPass) {
-                return bcryptHash(pass.value, 10).then(function (hash) {
+                hasher.update(bodyPass + salt, 'utf8');
-                    req.session.token = hash;
+                req.session.token = hasher.digest('hex');
+                req.session.salt = salt;
+
                 return res.redirect(config.urlFor({relativeUrl: decodeURI(req.body.forward)}));
-                });
             } else {
                 res.error = {
                     message: 'Wrong password'

core/test/unit/middleware_spec.js

@@ -1,15 +1,23 @@
 /*globals describe, beforeEach, afterEach, it*/
 /*jshint expr:true*/
 var assert          = require('assert'),
+    crypto          = require('crypto'),
     should          = require('should'),
     sinon           = require('sinon'),
     Promise         = require('bluebird'),
     middleware      = require('../../server/middleware').middleware,
     api             = require('../../server/api'),
     errors          = require('../../server/errors'),
-    bcrypt          = require('bcryptjs'),
     fs              = require('fs');
 
+function hash(password, salt) {
+    var hasher = crypto.createHash('sha256');
+
+    hasher.update(password + salt, 'utf8');
+
+    return hasher.digest('hex');
+}
+
 describe('Middleware', function () {
     var sandbox,
         apiSettingsStub;
@@ -272,8 +280,9 @@ describe('Middleware', function () {
             middleware.checkIsPrivate(req, res, next).then(function () {
                 next.called.should.be.true;
                 res.isPrivateBlog.should.be.false;
+
                 done();
-            });
+            }).catch(done);
         });
 
         it('checkIsPrivate should load session if private', function (done) {
@@ -286,8 +295,9 @@ describe('Middleware', function () {
 
             middleware.checkIsPrivate(req, res, next).then(function () {
                 res.isPrivateBlog.should.be.true;
+
                 done();
-            });
+            }).catch(done);
         });
 
         describe('not private', function () {
@@ -376,10 +386,6 @@ describe('Middleware', function () {
 
             describe('with hash verification', function () {
                 beforeEach(function () {
-                    sandbox.stub(bcrypt, 'compare', function (check, hash, cb) {
-                        var isVerified = (check === hash) ? true : false;
-                        cb(null, isVerified);
-                    });
                     apiSettingsStub.withArgs(sinon.match.has('key', 'password')).returns(Promise.resolve({
                         settings: [{
                             key: 'password',
@@ -389,69 +395,85 @@ describe('Middleware', function () {
                 });
 
                 it('authenticatePrivateSession should return next if hash is verified', function (done) {
+                    var salt = Date.now().toString();
+
                     req.session = {
-                        token: 'rightpassword'
+                        token: hash('rightpassword', salt),
+                        salt: salt
                     };
+
                     middleware.authenticatePrivateSession(req, res, next).then(function () {
                         next.called.should.be.true;
+
                         done();
-                    });
+                    }).catch(done);
                 });
 
                 it('authenticatePrivateSession should redirect if hash is not verified', function (done) {
                     req.url = '/welcome-to-ghost';
                     req.session = {
-                        token: 'wrongpassword'
+                        token: 'wrongpassword',
+                        salt: Date.now().toString()
                     };
                     res.redirect = sinon.spy();
+
                     middleware.authenticatePrivateSession(req, res, next).then(function () {
                         res.redirect.called.should.be.true;
+
                         done();
-                    });
+                    }).catch(done);
                 });
 
                 it('isPrivateSessionAuth should redirect if hash is verified', function (done) {
+                    var salt = Date.now().toString();
+
                     req.session = {
-                        token: 'rightpassword'
+                        token: hash('rightpassword', salt),
+                        salt: salt
                     };
                     res.redirect = sandbox.spy();
+
                     middleware.isPrivateSessionAuth(req, res, next).then(function () {
                         res.redirect.called.should.be.true;
+
                         done();
-                    });
+                    }).catch(done);
                 });
 
                 it('isPrivateSessionAuth should return next if hash is not verified', function (done) {
                     req.session = {
-                        token: 'wrongpassword'
+                        token: 'wrongpassword',
+                        salt: Date.now().toString()
                     };
+
                     middleware.isPrivateSessionAuth(req, res, next).then(function () {
                         next.called.should.be.true;
+
                         done();
-                    });
+                    }).catch(done);
                 });
 
                 it('authenticateProtection should return next if password is incorrect', function (done) {
                     req.body = {password: 'wrongpassword'};
+
                     middleware.authenticateProtection(req, res, next).then(function () {
                         res.error.should.not.be.empty;
                         next.called.should.be.true;
+
                         done();
-                    });
+                    }).catch(done);
                 });
 
                 it('authenticateProtection should redirect if password is correct', function (done) {
                     req.body = {password: 'rightpassword'};
                     req.session = {};
                     res.redirect = sandbox.spy();
-                    sandbox.stub(bcrypt, 'hash', function (pass, salt, cb) {
+
-                        cb(null, pass + 'hash');
-                    });
                     middleware.authenticateProtection(req, res, next).then(function () {
-                        req.session.token.should.equal('rightpasswordhash');
                         res.redirect.called.should.be.true;
+
                         done();
-                    });
+                    }).catch(done);
                 });
             });
         });