Merge pull request #2238 from gimelfarb/fix-1838-admin-ssl

Support for urlSSL config option and forceAdminSSL 403 response

core/server/config/url.js

@@ -26,17 +26,19 @@ function setConfig(config) {
 // Parameters:
 // - urlPath - string which must start and end with a slash
 // - absolute (optional, default:false) - boolean whether or not the url should be absolute
+// - secure (optional, default:false) - boolean whether or not to use urlSSL or url config
 // Returns:
 //  - a URL which always ends with a slash
-function createUrl(urlPath, absolute) {
+function createUrl(urlPath, absolute, secure) {
     urlPath = urlPath || '/';
     absolute = absolute || false;
 
-    var output = '';
+    var output = '', baseUrl;
 
     // create base of url, always ends without a slash
     if (absolute) {
-        output += ghostConfig.url.replace(/\/$/, '');
+        baseUrl = (secure && ghostConfig.urlSSL) ? ghostConfig.urlSSL : ghostConfig.url;
+        output += baseUrl.replace(/\/$/, '');
     } else {
         output += ghostConfig.paths.subdir;
     }
@@ -99,6 +101,7 @@ function urlPathForPost(post, permalinks) {
 // This is probably not the right place for this, but it's the best place for now
 function urlFor(context, data, absolute) {
     var urlPath = '/',
+        secure,
         knownObjects = ['post', 'tag', 'user'],
         knownPaths = {'home': '/', 'rss': '/rss/'}; // this will become really big
 
@@ -108,14 +111,19 @@ function urlFor(context, data, absolute) {
         data = null;
     }
 
+    // Can pass 'secure' flag in either context or data arg
+    secure = (context && context.secure) || (data && data.secure);
+
     if (_.isObject(context) && context.relativeUrl) {
         urlPath = context.relativeUrl;
     } else if (_.isString(context) && _.indexOf(knownObjects, context) !== -1) {
         // trying to create a url for an object
         if (context === 'post' && data.post && data.permalinks) {
             urlPath = urlPathForPost(data.post, data.permalinks);
+            secure = data.post.secure;
         } else if (context === 'tag' && data.tag) {
             urlPath = '/tag/' + data.tag.slug + '/';
+            secure = data.tag.secure;
         }
         // other objects are recognised but not yet supported
     } else if (_.isString(context) && _.indexOf(_.keys(knownPaths), context) !== -1) {
@@ -123,7 +131,7 @@ function urlFor(context, data, absolute) {
         urlPath = knownPaths[context] || '/';
     }
 
-    return createUrl(urlPath, absolute);
+    return createUrl(urlPath, absolute, secure);
 }
 
 // ## urlForPost

core/server/controllers/admin.js

@@ -301,8 +301,9 @@ adminControllers = {
         var email = req.body.email;
 
         api.users.generateResetToken(email).then(function (token) {
-            var siteLink = '<a href="' + config().url + '">' + config().url + '</a>',
+            var baseUrl = config().forceAdminSSL ? (config().urlSSL || config().url) : config().url,
-                resetUrl = config().url.replace(/\/$/, '') +  '/ghost/reset/' + token + '/',
+                siteLink = '<a href="' + baseUrl + '">' + baseUrl + '</a>',
+                resetUrl = baseUrl.replace(/\/$/, '') +  '/ghost/reset/' + token + '/',
                 resetLink = '<a href="' + resetUrl + '">' + resetUrl + '</a>',
                 message = {
                     to: email,

core/server/controllers/frontend.js

@@ -56,6 +56,14 @@ function handleError(next) {
     };
 }
 
+// Add Request context parameter to the data object
+// to be passed down to the templates
+function setReqCtx(req, data) {
+    (Array.isArray(data) ? data : [data]).forEach(function (d) {
+        d.secure = req.secure;
+    });
+}
+
 frontendControllers = {
     'homepage': function (req, res, next) {
         // Parse the page number
@@ -76,6 +84,8 @@ frontendControllers = {
                 return res.redirect(page.meta.pagination.pages === 1 ? config().paths.subdir + '/' : (config().paths.subdir + '/page/' + page.meta.pagination.pages + '/'));
             }
 
+            setReqCtx(req, page.posts);
+
             // Render the page of posts
             filters.doFilter('prePostsRender', page.posts).then(function (posts) {
                 res.render('index', formatPageResponse(posts, page));
@@ -113,6 +123,9 @@ frontendControllers = {
                 return res.redirect(tagUrl(options.tag, page.meta.pagination.pages));
             }
 
+            setReqCtx(req, page.posts);
+            setReqCtx(req, page.aspect.tag);
+
             // Render the page of posts
             filters.doFilter('prePostsRender', page.posts).then(function (posts) {
                 api.settings.read('activeTheme').then(function (activeTheme) {
@@ -184,6 +197,9 @@ frontendControllers = {
                     // Use throw 'no match' to show 404.
                     throw new Error('no match');
                 }
+
+                setReqCtx(req, post);
+
                 filters.doFilter('prePostsRender', post).then(function (post) {
                     api.settings.read('activeTheme').then(function (activeTheme) {
                         var paths = config().paths.availableThemes[activeTheme.value],
@@ -279,8 +295,8 @@ frontendControllers = {
                 var title = result[0].value.value,
                     description = result[1].value.value,
                     permalinks = result[2].value,
-                    siteUrl = config.urlFor('home', null, true),
+                    siteUrl = config.urlFor('home', {secure: req.secure}, true),
-                    feedUrl =  config.urlFor('rss', null, true),
+                    feedUrl =  config.urlFor('rss', {secure: req.secure}, true),
                     maxPage = page.meta.pagination.pages,
                     feedItems = [],
                     feed;
@@ -315,6 +331,8 @@ frontendControllers = {
                     }
                 }
 
+                setReqCtx(req, page.posts);
+
                 filters.doFilter('prePostsRender', page.posts).then(function (posts) {
                     posts.forEach(function (post) {
                         var deferred = when.defer(),

core/server/middleware/index.js

@@ -70,13 +70,24 @@ function ghostLocals(req, res, next) {
     }
 }
 
+function initThemeData(secure) {
+    var themeConfig = config.theme();
+    if (secure && config().urlSSL) {
+        // For secure requests override .url property with the SSL version
+        themeConfig = _.clone(themeConfig);
+        themeConfig.url = config().urlSSL.replace(/\/$/, '');
+    }
+    return themeConfig;
+}
+
 // ### InitViews Middleware
 // Initialise Theme or Admin Views
 function initViews(req, res, next) {
     /*jslint unparam:true*/
 
     if (!res.isAdmin) {
-        hbs.updateTemplateOptions({ data: {blog: config.theme()} });
+        var themeData = initThemeData(req.secure);
+        hbs.updateTemplateOptions({ data: {blog: themeData} });
         expressServer.engine('hbs', expressServer.get('theme view engine'));
         expressServer.set('views', path.join(config().paths.themePath, expressServer.get('activeTheme')));
     } else {
@@ -84,6 +95,10 @@ function initViews(req, res, next) {
         expressServer.set('views', config().paths.adminViews);
     }
 
+    // Pass 'secure' flag to the view engine
+    // so that templates can choose 'url' vs 'urlSSL'
+    res.locals.secure = req.secure;
+
     next();
 }
 
@@ -184,9 +199,20 @@ function isSSLrequired(isAdmin) {
 function checkSSL(req, res, next) {
     if (isSSLrequired(res.isAdmin)) {
         if (!req.secure) {
+            var forceAdminSSL = config().forceAdminSSL,
+                redirectUrl;
+
+            // Check if forceAdminSSL: { redirect: false } is set, which means
+            // we should just deny non-SSL access rather than redirect
+            if (forceAdminSSL && forceAdminSSL.redirect !== undefined && !forceAdminSSL.redirect) {
+                return res.send(403);
+            }
+
+            redirectUrl = url.parse(config().urlSSL || config().url);
             return res.redirect(301, url.format({
                 protocol: 'https:',
-                hostname: url.parse(config().url).hostname,
+                hostname: redirectUrl.hostname,
+                port: redirectUrl.port,
                 pathname: req.path,
                 query: req.query
             }));

core/test/functional/routes/admin_test.js

@@ -111,6 +111,80 @@ describe('Admin Routing', function () {
                 .end(doEndNoAuth(done));
         });
     });
+    
+    // we'll use X-Forwarded-Proto: https to simulate an 'https://' request behind a proxy
+    describe('Require HTTPS - no redirect', function() {
+        var forkedGhost, request;
+        before(function (done) {
+            var configTestHttps = testUtils.fork.config();
+            configTestHttps.forceAdminSSL = {redirect: false};
+            configTestHttps.urlSSL = 'https://localhost/';
+
+            testUtils.fork.ghost(configTestHttps, 'testhttps')
+                .then(function(child) {
+                    forkedGhost = child;
+                    request = require('supertest');
+                    request = request(configTestHttps.url.replace(/\/$/, ''));
+                }, done)
+                .then(done);
+        });
+        
+        after(function (done) {
+            if (forkedGhost) {
+                forkedGhost.kill(done);
+            }
+        });
+        
+        it('should block admin access over non-HTTPS', function(done) {
+            request.get('/ghost/')
+                .expect(403)
+                .end(done);
+        });
+
+        it('should allow admin access over HTTPS', function(done) {
+            request.get('/ghost/signup/')
+                .set('X-Forwarded-Proto', 'https')
+                .expect(200)
+                .end(doEnd(done));
+        });
+    });    
+
+    describe('Require HTTPS - redirect', function() {
+        var forkedGhost, request;
+        before(function (done) {
+            var configTestHttps = testUtils.fork.config();
+            configTestHttps.forceAdminSSL = {redirect: true};
+            configTestHttps.urlSSL = 'https://localhost/';
+
+            testUtils.fork.ghost(configTestHttps, 'testhttps')
+                .then(function(child) {
+                    forkedGhost = child;
+                    request = require('supertest');
+                    request = request(configTestHttps.url.replace(/\/$/, ''));
+                }, done)
+                .then(done);
+        });
+        
+        after(function (done) {
+            if (forkedGhost) {
+                forkedGhost.kill(done);
+            }
+        });
+        
+        it('should redirect admin access over non-HTTPS', function(done) {
+            request.get('/ghost/')
+                .expect('Location', /^https:\/\/localhost\/ghost\//)
+                .expect(301)
+                .end(done);
+        });
+
+        it('should allow admin access over HTTPS', function(done) {
+            request.get('/ghost/signup/')
+                .set('X-Forwarded-Proto', 'https')
+                .expect(200)
+                .end(done);
+        });
+    });    
 
     describe('Ghost Admin Signup', function () {
         it('should have a session cookie which expires in 12 hours', function (done) {

core/test/functional/routes/frontend_test.js

@@ -9,6 +9,7 @@ var request    = require('supertest'),
     express    = require('express'),
     should     = require('should'),
     moment     = require('moment'),
+    path       = require('path'),
 
     testUtils  = require('../../utils'),
     ghost      = require('../../../../core'),
@@ -142,6 +143,47 @@ describe('Frontend Routing', function () {
                 .end(doEnd(done));
         });
     });
+    
+    // we'll use X-Forwarded-Proto: https to simulate an 'https://' request behind a proxy
+    describe('HTTPS', function() {
+        var forkedGhost, request;
+        before(function (done) {
+            var configTestHttps = testUtils.fork.config();
+            configTestHttps.forceAdminSSL = {redirect: false};
+            configTestHttps.urlSSL = 'https://localhost/';
+
+            testUtils.fork.ghost(configTestHttps, 'testhttps')
+                .then(function(child) {
+                    forkedGhost = child;
+                    request = require('supertest');
+                    request = request(configTestHttps.url.replace(/\/$/, ''));
+                }, done)
+                .then(done);
+        });
+        
+        after(function (done) {
+            if (forkedGhost) {
+                forkedGhost.kill(done);
+            }
+        });
+        
+        it('should set links to url over non-HTTPS', function(done) {
+            request.get('/')
+                .expect(200)
+                .expect(/\<link rel="canonical" href="http:\/\/127.0.0.1:2370\/" \/\>/)
+                .expect(/copyright \<a href="http:\/\/127.0.0.1:2370\/">Ghost\<\/a\>/)
+                .end(doEnd(done));
+        });
+
+        it('should set links to urlSSL over HTTPS', function(done) {
+            request.get('/')
+                .set('X-Forwarded-Proto', 'https')
+                .expect(200)
+                .expect(/\<link rel="canonical" href="https:\/\/localhost\/" \/\>/)
+                .expect(/copyright \<a href="https:\/\/localhost\/">Ghost\<\/a\>/)
+                .end(doEnd(done));
+        });
+    });
 
     describe('RSS', function () {
         it('should redirect without slash', function (done) {

core/test/utils/fork.js

@@ -0,0 +1,128 @@
+var cp         = require('child_process'),
+    _          = require('lodash'),
+    fs         = require('fs'),
+    url        = require('url'),
+    net        = require('net'),
+    when       = require('when'),
+    path       = require('path'),
+    config     = require('../../server/config');
+    
+function findFreePort(port) {
+    var deferred = when.defer();
+
+    if (typeof port === 'string') port = parseInt(port);
+    if (typeof port !== 'number') port = 2368;
+    port = port + 1;
+
+    var server = net.createServer();
+    server.on('error', function(e) {
+        if (e.code === 'EADDRINUSE') {
+            when.chain(findFreePort(port), deferred);
+        } else {
+            deferred.reject(e);
+        }
+    });
+    server.listen(port, function() {
+        var listenPort = server.address().port;
+        server.close(function() {
+            deferred.resolve(listenPort);
+        });
+    });
+    
+    return deferred.promise;
+}
+
+// Get a copy of current config object from file, to be modified before
+// passing to forkGhost() method
+function forkConfig() {
+    // require caches values, and we want to read it fresh from the file
+    delete require.cache[config().paths.config];
+    return _.cloneDeep(require(config().paths.config)[process.env.NODE_ENV]);
+}
+
+// Creates a new fork of Ghost process with a given config
+// Useful for tests that want to verify certain config options
+function forkGhost(newConfig, envName) {
+    var deferred = when.defer();
+    envName = envName || 'forked';
+    findFreePort(newConfig.server ? newConfig.server.port : undefined)
+        .then(function(port) {
+            newConfig.server = newConfig.server || {};
+            newConfig.server.port = port;
+            newConfig.url = url.format(_.extend(url.parse(newConfig.url), {port: port, host: null}));
+            
+            var newConfigFile = path.join(config().paths.appRoot, 'config.test' + port + '.js');
+            fs.writeFile(newConfigFile, 'module.exports = {' + envName + ': ' + JSON.stringify(newConfig) + '}', function(err) {
+                if (err) throw err;
+                
+                // setup process environment for the forked Ghost to use the new config file
+                var env = _.clone(process.env);
+                env['GHOST_CONFIG'] = newConfigFile;
+                env['NODE_ENV'] = envName;
+                var child = cp.fork(path.join(config().paths.appRoot, 'index.js'), {env: env});
+                
+                var pingTries = 0;
+                var pingCheck;
+                var pingStop = function() {
+                    if (pingCheck) {
+                        clearInterval(pingCheck);
+                        pingCheck = undefined;
+                        return true;
+                    }
+                    return false;
+                };
+                // periodic check until forked Ghost is running and is listening on the port
+                pingCheck = setInterval(function() {
+                    var socket = net.connect(port);
+                    socket.on('connect', function() {
+                        socket.end();
+                        if (pingStop()) {
+                            deferred.resolve(child);
+                        }
+                    });
+                    socket.on('error', function(err) {
+                        // continue checking
+                        if (++pingTries >= 20 && pingStop()) {
+                            deferred.reject(new Error("Timed out waiting for child process"));
+                        }
+                    });
+                }, 200);
+        
+                child.on('exit', function(code, signal) {
+                    child.exited = true;
+                    if (pingStop()) {
+                        deferred.reject(new Error("Child process exit code: " + code));
+                    }
+                    // cleanup the temporary config file
+                    fs.unlink(newConfigFile);
+                });
+                
+                // override kill() to have an async callback
+                var baseKill = child.kill;
+                child.kill = function(signal, cb) {
+                    if (typeof signal === 'function') {
+                        cb = signal;
+                        signal = undefined;
+                    }
+                    
+                    if (cb) {
+                        child.on('exit', function() {
+                            cb();
+                        });
+                    }
+
+                    if (child.exited) {
+                        process.nextTick(cb);
+                    } else {
+                        baseKill.apply(child, [signal]);
+                    }
+                };
+            });
+        })
+        .otherwise(deferred.reject);
+
+    return deferred.promise;
+}
+
+module.exports.ghost = forkGhost;
+module.exports.config = forkConfig;

core/test/utils/index.js

@@ -8,7 +8,8 @@ var knex          = require('../../server/models/base').knex,
     migration     = require("../../server/data/migration/"),
     Settings      = require('../../server/models/settings').Settings,
     DataGenerator = require('./fixtures/data-generator'),
-    API           = require('./api');
+    API           = require('./api'),
+    fork          = require('./fork');
 
 function initData() {
     return migration.init();
@@ -228,5 +229,7 @@ module.exports = {
     loadExportFixture: loadExportFixture,
 
     DataGenerator: DataGenerator,
-    API: API
+    API: API,
+    
+    fork: fork
 };