From bc25348fccdb860b96617ba6c3351716df6644d5 Mon Sep 17 00:00:00 2001 From: Sebastian Gierlinger Date: Fri, 25 Sep 2015 12:02:14 +0200 Subject: [PATCH] SSL redirects closes #5873 - replaced redirectPathname with url method - added tests --- core/server/middleware/check-ssl.js | 14 +----- core/test/unit/middleware/check-ssl_spec.js | 47 ++++++++++++++++++++- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/core/server/middleware/check-ssl.js b/core/server/middleware/check-ssl.js index 85f14d1ee8..60797a3a52 100644 --- a/core/server/middleware/check-ssl.js +++ b/core/server/middleware/check-ssl.js @@ -14,29 +14,19 @@ function isSSLrequired(isAdmin, configUrl, forceAdminSSL) { // 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 + reqUrl = url.parse(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 redirectPathname() { - var pathname = baseUrl.path; - if (reqUrl[0] === '/' && pathname[pathname.length - 1] === '/') { - pathname += reqUrl.slice(1); - } else { - pathname += reqUrl; - } - return pathname; - }, redirectUrl: function redirectUrl(query) { return url.format({ protocol: 'https:', hostname: baseUrl.hostname, port: baseUrl.port, - pathname: this.redirectPathname(), + pathname: reqUrl.pathname, query: query }); } diff --git a/core/test/unit/middleware/check-ssl_spec.js b/core/test/unit/middleware/check-ssl_spec.js index a1c430bab7..33a674e08b 100644 --- a/core/test/unit/middleware/check-ssl_spec.js +++ b/core/test/unit/middleware/check-ssl_spec.js @@ -104,8 +104,53 @@ describe('checkSSL', function () { done(); }); + it('should redirect to subdirectory with force admin SSL (admin)', function (done) { + req.url = '/blog/ghost/'; + res.isAdmin = true; + res.redirect = {}; + req.secure = false; + config.set({ + url: 'http://default.com:2368/blog/', + 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/blog/ghost/'); + return; + }); + checkSSL(req, res, next); + next.called.should.be.false; + done(); + }); + + it('should redirect and keep query with force admin SSL (admin)', function (done) { + req.url = '/ghost/'; + req.query = { + test: 'true' + }; + 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/?test=true'); + 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.url = '/'; req.secure = false; res.redirect = {}; config.set({