From 762824690af99d9d4e6dfeee928d9d1d92e5d83a Mon Sep 17 00:00:00 2001 From: Sebastian Gierlinger Date: Tue, 8 Sep 2015 18:05:22 +0200 Subject: [PATCH] Improve Middleware Code Coverage refs #5286 - finished tests for check-ssl.js --- core/server/middleware/check-ssl.js | 10 +- core/test/unit/middleware/check-ssl_spec.js | 220 +++++++++++++------- 2 files changed, 151 insertions(+), 79 deletions(-) diff --git a/core/server/middleware/check-ssl.js b/core/server/middleware/check-ssl.js index 02f885d699..85f14d1ee8 100644 --- a/core/server/middleware/check-ssl.js +++ b/core/server/middleware/check-ssl.js @@ -1,5 +1,6 @@ var config = require('../config'), - url = require('url'); + url = require('url'), + checkSSL; function isSSLrequired(isAdmin, configUrl, forceAdminSSL) { var forceSSL = url.parse(configUrl).protocol === 'https:' ? true : false; @@ -46,7 +47,7 @@ function sslForbiddenOrRedirect(opt) { // Check to see if we should use SSL // and redirect if needed -function checkSSL(req, res, next) { +checkSSL = function checkSSL(req, res, next) { if (isSSLrequired(res.isAdmin, config.url, config.forceAdminSSL)) { if (!req.secure) { var response = sslForbiddenOrRedirect({ @@ -64,9 +65,6 @@ function checkSSL(req, res, next) { } } next(); -} +}; module.exports = checkSSL; -// SSL helper functions are exported primarily for unit testing. -module.exports.isSSLrequired = isSSLrequired; -module.exports.sslForbiddenOrRedirect = sslForbiddenOrRedirect; diff --git a/core/test/unit/middleware/check-ssl_spec.js b/core/test/unit/middleware/check-ssl_spec.js index 9bdfcad5cd..a1c430bab7 100644 --- a/core/test/unit/middleware/check-ssl_spec.js +++ b/core/test/unit/middleware/check-ssl_spec.js @@ -1,92 +1,166 @@ /*globals describe, it, beforeEach, afterEach */ /*jshint expr:true*/ var sinon = require('sinon'), + should = require('should'), + config = require('../../../server/config'), checkSSL = require('../../../server/middleware/check-ssl'); +should.equal(true, true); + describe('checkSSL', function () { - var sandbox, res, req, next; + var res, req, next, sandbox; beforeEach(function () { sandbox = sinon.sandbox.create(); - req = sinon.spy(); - res = sinon.spy(); - next = sinon.spy(); + req = {}; + res = {}; + next = sandbox.spy(); }); afterEach(function () { sandbox.restore(); }); - it('skips if already on SSL', function () { - res.isAdmin = true; - req.isSecure = true; + it('should not require SSL (frontend)', function (done) { + req.url = '/'; + config.set({ + url: 'http://default.com:2368/' + }); checkSSL(req, res, next); next.called.should.be.true; - }); -}); - -describe('isSSLRequired', function () { - var isSSLrequired = checkSSL.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 = checkSSL.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'); + next.calledWith().should.be.true; + done(); + }); + + it('should require SSL (frontend)', function (done) { + req.url = '/'; + req.secure = true; + config.set({ + url: 'https://default.com:2368/' + }); + checkSSL(req, res, next); + next.called.should.be.true; + next.calledWith().should.be.true; + done(); + }); + + it('should not require SSL (admin)', function (done) { + req.url = '/ghost'; + res.isAdmin = true; + config.set({ + url: 'http://default.com:2368/' + }); + checkSSL(req, res, next); + next.called.should.be.true; + next.calledWith().should.be.true; + done(); + }); + + it('should not redirect with SSL (admin)', function (done) { + req.url = '/ghost'; + res.isAdmin = true; + res.secure = true; + config.set({ + url: 'http://default.com:2368/' + }); + checkSSL(req, res, next); + next.called.should.be.true; + next.calledWith().should.be.true; + done(); + }); + + it('should not redirect with force admin SSL (admin)', function (done) { + req.url = '/ghost'; + res.isAdmin = true; + req.secure = true; + config.set({ + url: 'http://default.com:2368/', + forceAdminSSL: true + }); + checkSSL(req, res, next); + next.called.should.be.true; + next.calledWith().should.be.true; + done(); + }); + + it('should redirect with force admin SSL (admin)', function (done) { + req.url = '/ghost/'; + res.isAdmin = true; + res.redirect = {}; + req.secure = false; + config.set({ + url: 'http://default.com:2368/', + urlSSL: '', + forceAdminSSL: true + }); + sandbox.stub(res, 'redirect', function (statusCode, url) { + statusCode.should.eql(301); + url.should.not.be.empty; + url.should.eql('https://default.com:2368/ghost/'); + return; + }); + checkSSL(req, res, next); + next.called.should.be.false; + done(); + }); + + it('should redirect with with config.url being SSL (frontend)', function (done) { + req.url = ''; + req.secure = false; + res.redirect = {}; + config.set({ + url: 'https://default.com:2368', + urlSSL: '', + forceAdminSSL: true + }); + sandbox.stub(res, 'redirect', function (statusCode, url) { + statusCode.should.eql(301); + url.should.not.be.empty; + url.should.eql('https://default.com:2368/'); + return; + }); + checkSSL(req, res, next); + next.called.should.be.false; + done(); + }); + + it('should redirect to urlSSL (admin)', function (done) { + req.url = '/ghost/'; + res.isAdmin = true; + res.redirect = {}; + req.secure = false; + config.set({ + url: 'http://default.com:2368/', + urlSSL: 'https://ssl-domain.com:2368/' + }); + sandbox.stub(res, 'redirect', function (statusCode, url) { + statusCode.should.eql(301); + url.should.not.be.empty; + url.should.eql('https://ssl-domain.com:2368/ghost/'); + return; + }); + checkSSL(req, res, next); + next.called.should.be.false; + done(); + }); + + it('should not redirect if redirect:false (admin)', function (done) { + req.url = '/ghost/'; + res.isAdmin = true; + res.sendStatus = {}; + req.secure = false; + config.set({ + url: 'http://default.com:2368/', + forceAdminSSL: { + redirect: false + } + }); + sandbox.stub(res, 'sendStatus', function (statusCode) { + statusCode.should.eql(403); + return; + }); + checkSSL(req, res, next); + next.called.should.be.false; + done(); }); });