diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index 601f7a0c4e..e727dbc617 100644 --- a/core/server/middleware/index.js +++ b/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,51 +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, - configUrl, - 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); - } - - configUrl = url.parse(config.urlSSL || config.url); - - redirectUrl = configUrl.path; - if (req.url[0] === '/' && redirectUrl[redirectUrl.length - 1] === '/') { - redirectUrl += req.url.slice(1); - } else { - redirectUrl += req.url; - } - - return res.redirect(301, url.format({ - protocol: 'https:', - hostname: configUrl.hostname, - port: configUrl.port, - pathname: redirectUrl, - query: req.query - })); - } - } - next(); -} - // ### ServeSharedFile Middleware // Handles requests to robots.txt and favicon.ico (and caches them) function serveSharedFile(file, type, maxAge) { @@ -296,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 diff --git a/core/server/middleware/middleware.js b/core/server/middleware/middleware.js index 04962ad4c1..5b3b444937 100644 --- a/core/server/middleware/middleware.js +++ b/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; diff --git a/core/test/unit/middleware_spec.js b/core/test/unit/middleware_spec.js index 0e19320d1f..95c10e976b 100644 --- a/core/test/unit/middleware_spec.js +++ b/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'); + }); + }); });