Merge pull request #4821 from markstos/https-subdirectory

Fixes #4435, also refactors checkSSL to be unit-tested.

core/server/middleware/index.js

@@ -16,7 +16,6 @@ var api            = require('../api'),
     routes         = require('../routes'),
     slashes        = require('connect-slashes'),
     storage        = require('../storage'),
-    url            = require('url'),
     _              = require('lodash'),
     passport       = require('passport'),
     oauth          = require('./oauth'),
@@ -163,42 +162,6 @@ function uncapitalise(req, res, next) {
     }
 }
 
-function isSSLrequired(isAdmin) {
-    var forceSSL = url.parse(config.url).protocol === 'https:' ? true : false,
-        forceAdminSSL = (isAdmin && config.forceAdminSSL);
-    if (forceSSL || forceAdminSSL) {
-        return true;
-    }
-    return false;
-}
-
-// Check to see if we should use SSL
-// and redirect if needed
-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.sendStatus(403);
-            }
-
-            redirectUrl = url.parse(config.urlSSL || config.url);
-            return res.redirect(301, url.format({
-                protocol: 'https:',
-                hostname: redirectUrl.hostname,
-                port: redirectUrl.port,
-                pathname: req.path,
-                query: req.query
-            }));
-        }
-    }
-    next();
-}
-
 // ### ServeSharedFile Middleware
 // Handles requests to robots.txt and favicon.ico (and caches them)
 function serveSharedFile(file, type, maxAge) {
@@ -287,7 +250,7 @@ setupMiddleware = function (blogAppInstance, adminApp) {
     // NOTE: Importantly this is _after_ the check above for admin-theme static resources,
     //       which do not need HTTPS. In fact, if HTTPS is forced on them, then 404 page might
     //       not display properly when HTTPS is not available!
-    blogApp.use(checkSSL);
+    blogApp.use(middleware.checkSSL);
     adminApp.set('views', config.paths.adminViews);
 
     // Theme only config

core/server/middleware/middleware.js

@@ -10,6 +10,7 @@ var _           = require('lodash'),
     api         = require('../api'),
     passport    = require('passport'),
     errors      = require('../errors'),
+    url         = require('url'),
     utils       = require('../utils'),
 
     middleware,
@@ -32,6 +33,49 @@ function cacheOauthServer(server) {
     oauthServer = server;
 }
 
+function isSSLrequired(isAdmin, configUrl, forceAdminSSL) {
+    var forceSSL = url.parse(configUrl).protocol === 'https:' ? true : false;
+    if (forceSSL || (isAdmin && forceAdminSSL)) {
+        return true;
+    }
+    return false;
+}
+
+// The guts of checkSSL. Indicate forbidden or redirect according to configuration.
+// Required args: forceAdminSSL, url and urlSSL should be passed from config. reqURL from req.url
+function sslForbiddenOrRedirect(opt) {
+    var forceAdminSSL = opt.forceAdminSSL,
+        reqUrl        = opt.reqUrl, // expected to be relative-to-root
+        baseUrl       = url.parse(opt.configUrlSSL || opt.configUrl),
+        response = {
+        // Check if forceAdminSSL: { redirect: false } is set, which means
+        // we should just deny non-SSL access rather than redirect
+        isForbidden: (forceAdminSSL && forceAdminSSL.redirect !== undefined && !forceAdminSSL.redirect),
+
+        // Append the request path to the base configuration path, trimming out a double "//"
+        redirectPathname: function () {
+            var pathname  = baseUrl.path;
+            if (reqUrl[0] === '/' && pathname[pathname.length - 1] === '/') {
+                pathname += reqUrl.slice(1);
+            } else {
+                pathname += reqUrl;
+            }
+            return pathname;
+        },
+        redirectUrl: function (query) {
+            return url.format({
+                protocol: 'https:',
+                hostname: baseUrl.hostname,
+                port: baseUrl.port,
+                pathname: this.redirectPathname(),
+                query: query
+            });
+        }
+    };
+
+    return response;
+}
+
 middleware = {
 
     // ### Authenticate Middleware
@@ -259,9 +303,35 @@ middleware = {
         return oauthServer.token()(req, res, next);
     },
 
+    // Check to see if we should use SSL
+    // and redirect if needed
+    checkSSL: function (req, res, next) {
+        if (isSSLrequired(res.isAdmin, config.url, config.forceAdminSSL)) {
+            if (!req.secure) {
+                var response = sslForbiddenOrRedirect({
+                    forceAdminSSL: config.forceAdminSSL,
+                    configUrlSSL: config.urlSSL,
+                    configUrl: config.url,
+                    reqUrl: req.url
+                });
+
+                if (response.isForbidden) {
+                    return res.sendStatus(403);
+                } else {
+                    return res.redirect(301, response.redirectUrl(req.query));
+                }
+            }
+        }
+        next();
+    },
+
     busboy: busboy
 };
 
 module.exports = middleware;
 module.exports.cacheBlogApp = cacheBlogApp;
 module.exports.cacheOauthServer = cacheOauthServer;
+
+// SSL helper functions are exported primarily for unity testing.
+module.exports.isSSLrequired = isSSLrequired;
+module.exports.sslForbiddenOrRedirect = sslForbiddenOrRedirect;

core/test/unit/middleware_spec.js

@@ -170,4 +170,70 @@ describe('Middleware', function () {
             });
         });
     });
+
+    describe('isSSLRequired', function () {
+        var isSSLrequired = middleware.isSSLrequired;
+
+        it('SSL is required if config.url starts with https', function () {
+            isSSLrequired(undefined, 'https://example.com', undefined).should.be.true;
+        });
+
+        it('SSL is required if isAdmin and config.forceAdminSSL is set', function () {
+            isSSLrequired(true, 'http://example.com', true).should.be.true;
+        });
+
+        it('SSL is not required if config.url starts with "http:/" and forceAdminSSL is not set', function () {
+            isSSLrequired(false, 'http://example.com', false).should.be.false;
+        });
+    });
+
+    describe('sslForbiddenOrRedirect', function () {
+        var sslForbiddenOrRedirect = middleware.sslForbiddenOrRedirect;
+        it('Return forbidden if config forces admin SSL for AdminSSL redirect is false.', function () {
+            var response = sslForbiddenOrRedirect({
+                forceAdminSSL: {redirect: false},
+                configUrl: 'http://example.com'
+            });
+            response.isForbidden.should.be.true;
+        });
+
+        it('If not forbidden, should produce SSL to redirect to when config.url ends with no slash', function () {
+            var response = sslForbiddenOrRedirect({
+                forceAdminSSL: {redirect: true},
+                configUrl: 'http://example.com/config/path',
+                reqUrl: '/req/path'
+            });
+            response.isForbidden.should.be.false;
+            response.redirectUrl({}).should.equal('https://example.com/config/path/req/path');
+        });
+
+        it('If config ends is slash, potential double-slash in resulting URL is removed', function () {
+            var response = sslForbiddenOrRedirect({
+                forceAdminSSL: {redirect: true},
+                configUrl: 'http://example.com/config/path/',
+                reqUrl: '/req/path'
+            });
+            response.redirectUrl({}).should.equal('https://example.com/config/path/req/path');
+        });
+
+        it('If config.urlSSL is provided it is preferred over config.url', function () {
+            var response = sslForbiddenOrRedirect({
+                forceAdminSSL: {redirect: true},
+                configUrl: 'http://example.com/config/path/',
+                configUrlSSL: 'https://example.com/ssl/config/path/',
+                reqUrl: '/req/path'
+            });
+            response.redirectUrl({}).should.equal('https://example.com/ssl/config/path/req/path');
+        });
+
+        it('query string in request is preserved in redirect URL', function () {
+            var response = sslForbiddenOrRedirect({
+                forceAdminSSL: {redirect: true},
+                configUrl: 'http://example.com/config/path/',
+                configUrlSSL: 'https://example.com/ssl/config/path/',
+                reqUrl: '/req/path'
+            });
+            response.redirectUrl({a: 'b'}).should.equal('https://example.com/ssl/config/path/req/path?a=b');
+        });
+    });
 });