Refactor: Make checkSSL unit-testable and add unit tests for it.
- Code was moved to core/server/middleware/middleware.js, which is the home for unit-testable middleware. - Functional code coverage for this code also exists at: test/functional/routes/admin_test.js
This commit is contained in:
parent
094d6dfc38
commit
770317b834
@ -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
|
||||
|
@ -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;
|
||||
|
@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user