🎹 re-work url redirects (#7956)

refs #7488

- we have recently changed our url redirects
- see https://github.com/TryGhost/Ghost/pull/7937
- the url has a canonical meaning and that's why Ghost shouldn't force redirect to the blog url

core/server/middleware/url-redirects.js

@@ -1,9 +1,10 @@
 var url = require('url'),
-    debug = require('debug')('ghost:redirects'),
+    debug = require('debug')('ghost:url-redirects'),
     utils = require('../utils'),
-    urlRedirects;
+    urlRedirects,
+    _private = {};
 
-function redirectUrl(options) {
+_private.redirectUrl = function redirectUrl(options) {
     var redirectTo = options.redirectTo,
         path = options.path,
         query = options.query,
@@ -16,50 +17,84 @@ function redirectUrl(options) {
         pathname: path,
         query: query
     });
-}
+};
 
-/**
+_private.getAdminRedirectUrl = function getAdminRedirectUrl(options) {
- * SSL AND REDIRECTS
+    var admimHostWithoutProtocol = utils.url.urlFor('admin', {cors: true}, true),
- */
+        adminHostWithProtocol = utils.url.urlFor('admin', true),
-urlRedirects = function urlRedirects(req, res, next) {
+        requestedHost = options.requestedHost,
-    var requestedUrl = req.originalUrl || req.url,
+        requestedUrl = options.requestedUrl,
-        requestedHost = req.get('host'),
+        queryParameters = options.queryParameters,
-        targetHostWithProtocol,
+        secure = options.secure;
-        targetHostWithoutProtocol;
-
-    if (res.isAdmin) {
-        targetHostWithProtocol = utils.url.urlFor('admin', true);
-        targetHostWithoutProtocol = utils.url.urlFor('admin', {cors: true}, true);
-    } else {
-        targetHostWithProtocol = utils.url.urlFor('home', true);
-        targetHostWithoutProtocol = utils.url.urlFor('home', {cors: true}, true);
-    }
 
     debug('requestedUrl', requestedUrl);
     debug('requestedHost', requestedHost);
-    debug('targetHost', targetHostWithoutProtocol);
+    debug('adminHost', adminHostWithProtocol);
 
-    // CASE: custom admin url is configured, but user requests blog domain
+    // CASE: we always redirect to the correct admin url, if configured
-    // CASE: exception: localhost is always allowed
+    if (!admimHostWithoutProtocol.match(new RegExp(requestedHost))) {
-    if (!targetHostWithoutProtocol.match(new RegExp(requestedHost))) {
         debug('redirect because host does not match');
 
-        return res.redirect(301, redirectUrl({
+        return _private.redirectUrl({
-            redirectTo: targetHostWithProtocol,
+            redirectTo: adminHostWithProtocol,
             path: requestedUrl,
-            query: req.query
+            query: queryParameters
-        }));
+        });
     }
 
-    // CASE: correct admin url, but not the correct protocol
+    // CASE: configured admin url is HTTPS, but request is HTTP
-    if (utils.url.isSSL(targetHostWithProtocol) && !req.secure) {
+    if (utils.url.isSSL(adminHostWithProtocol) && !secure) {
         debug('redirect because protocol does not match');
 
-        return res.redirect(301, redirectUrl({
+        return _private.redirectUrl({
-            redirectTo: targetHostWithProtocol,
+            redirectTo: adminHostWithProtocol,
             path: requestedUrl,
-            query: req.query
+            query: queryParameters
-        }));
+        });
+    }
+};
+
+_private.getBlogRedirectUrl = function getBlogRedirectUrl(options) {
+    var blogHostWithProtocol = utils.url.urlFor('home', true),
+        requestedHost = options.requestedHost,
+        requestedUrl = options.requestedUrl,
+        queryParameters = options.queryParameters,
+        secure = options.secure;
+
+    debug('requestedUrl', requestedUrl);
+    debug('requestedHost', requestedHost);
+    debug('blogHost', blogHostWithProtocol);
+
+    // CASE: configured canonical url is HTTPS, but request is HTTP, redirect to requested host + SSL
+    if (utils.url.isSSL(blogHostWithProtocol) && !secure) {
+        debug('redirect because protocol does not match');
+
+        return _private.redirectUrl({
+            redirectTo: 'https://' + requestedHost,
+            path: requestedUrl,
+            query: queryParameters
+        });
+    }
+};
+
+/**
+ * Takes care of
+ *
+ * 1. required SSL redirects
+ * 2. redirect to the correct admin url
+ */
+urlRedirects = function urlRedirects(req, res, next) {
+    var redirectFn = res.isAdmin ? _private.getAdminRedirectUrl : _private.getBlogRedirectUrl,
+        redirectUrl = redirectFn({
+            requestedHost: req.get('host'),
+            requestedUrl: req.originalUrl || req.url,
+            queryParameters: req.query,
+            secure: req.secure
+        });
+
+    if (redirectUrl) {
+        debug('url redirect to: ' + redirectUrl);
+        return res.redirect(301, redirectUrl);
     }
 
     debug('no url redirect');

core/test/unit/middleware/url-redirects_spec.js

@@ -59,7 +59,7 @@ describe('checkSSL', function () {
         done();
     });
 
-    it('blog is https, requester uses http [redirect]', function (done) {
+    it('[redirect] blog is https, requester uses http', function (done) {
         configUtils.set({
             url: 'https://default.com:2368/'
         });
@@ -70,6 +70,7 @@ describe('checkSSL', function () {
         urlRedirects(req, res, next);
         next.called.should.be.false();
         res.redirect.called.should.be.true();
+        res.redirect.calledWith(301, 'https://default.com:2368/').should.be.true();
         done();
     });
 
@@ -103,6 +104,36 @@ describe('checkSSL', function () {
         done();
     });
 
+    it('blog host is !== request host', function (done) {
+        configUtils.set({
+            url: 'https://default.com'
+        });
+
+        host = 'localhost:2368';
+
+        req.originalUrl = '/';
+        req.secure = true;
+        urlRedirects(req, res, next);
+        next.called.should.be.true();
+        res.redirect.called.should.be.false();
+        done();
+    });
+
+    it('[redirect] blog host is !== request host', function (done) {
+        configUtils.set({
+            url: 'https://default.com'
+        });
+
+        host = 'localhost:2368';
+
+        req.originalUrl = '/';
+        urlRedirects(req, res, next);
+        next.called.should.be.false();
+        res.redirect.called.should.be.true();
+        res.redirect.calledWith(301, 'https://localhost:2368/').should.be.true();
+        done();
+    });
+
     it('admin is blog url and http, requester is http', function (done) {
         configUtils.set({
             url: 'http://default.com:2368'
@@ -118,7 +149,7 @@ describe('checkSSL', function () {
         done();
     });
 
-    it('admin is custom url and https, requester is http [redirect]', function (done) {
+    it('[redirect] admin is custom url and https, requester is http', function (done) {
         configUtils.set({
             url: 'http://default.com:2368',
             admin: {
@@ -136,7 +167,7 @@ describe('checkSSL', function () {
         done();
     });
 
-    it('admin is custom url and https, requester is http [redirect]', function (done) {
+    it('[redirect] admin is custom url and https, requester is http', function (done) {
         configUtils.set({
             url: 'http://default.com:2368',
             admin: {
@@ -154,7 +185,7 @@ describe('checkSSL', function () {
         done();
     });
 
-    it('subdirectory [redirect]', function (done) {
+    it('[redirect] subdirectory', function (done) {
         configUtils.set({
             url: 'http://default.com:2368/blog',
             admin: {
@@ -172,7 +203,7 @@ describe('checkSSL', function () {
         done();
     });
 
-    it('keeps query [redirect]', function (done) {
+    it('[redirect] keeps query', function (done) {
         configUtils.set({
             url: 'http://default.com:2368',
             admin: {