From 4a67ea55460d163c46a8c4a6d5d623929cdf7296 Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Thu, 23 Apr 2020 19:00:28 +0100
Subject: [PATCH] Fixed admin host with port causing infinite redirect (#11767)

closes #11766, refs 7284227f1

- when we changed from host to hostname, more changed than just using the x-forwarded-host if trusted because express req.hostname does not return the port
- this causes issues with an infinite redirect if you try to set a different admin host with a port
- added a test to demonstrate the case, that didn't fail due to an error in the test logic
- switched from redirecting based on req.hostname to using req.vhost.host which has the correct trusted, requested value that we should rely on
- simplified the comparison logic to explicitly compare host with host
---
 .../web/shared/middlewares/url-redirects.js   | 14 +++++------
 .../unit/web/middleware/url-redirects_spec.js | 23 +++++++++++++++++--
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/core/server/web/shared/middlewares/url-redirects.js b/core/server/web/shared/middlewares/url-redirects.js
index bea4fc1efe..b92a6c975f 100644
--- a/core/server/web/shared/middlewares/url-redirects.js
+++ b/core/server/web/shared/middlewares/url-redirects.js
@@ -33,17 +33,17 @@ _private.redirectUrl = ({redirectTo, query, pathname}) => {
 _private.getAdminRedirectUrl = ({requestedHost, requestedUrl, queryParameters, secure}) => {
     const siteUrl = urlUtils.urlFor('home', true);
     const adminUrl = urlUtils.urlFor('admin', true);
-    const adminUrlWithoutProtocol = adminUrl.replace(/(^\w+:|^)\/\//, '');
-    const siteUrlWithoutProtocol = siteUrl.replace(/(^\w+:|^)\/\//, '');
+    const siteHost = url.parse(siteUrl).host;
+    const adminHost = url.parse(adminUrl).host;
 
-    debug('getAdminRedirectUrl', requestedHost, requestedUrl, adminUrlWithoutProtocol, siteUrlWithoutProtocol, urlUtils.urlJoin(siteUrlWithoutProtocol, 'ghost/'));
+    debug('getAdminRedirectUrl', requestedHost, requestedUrl, adminHost, siteHost);
 
     // CASE: we only redirect the admin access if `admin.url` is configured
     // If url and admin.url are not equal AND the requested host does not match, redirect.
     // The first condition is the most important, because it ensures that you have a custom admin url configured,
     // because we don't force an admin redirect if you have a custom url configured, but no admin url.
-    if (adminUrlWithoutProtocol !== urlUtils.urlJoin(siteUrlWithoutProtocol, 'ghost/') &&
-        adminUrlWithoutProtocol !== urlUtils.urlJoin(requestedHost, urlUtils.getSubdir(), 'ghost/')) {
+    if (adminHost !== siteHost &&
+        adminHost !== requestedHost) {
         debug('redirect because admin host does not match');
 
         return _private.redirectUrl({
@@ -73,7 +73,7 @@ _private.getAdminRedirectUrl = ({requestedHost, requestedUrl, queryParameters, s
 _private.getFrontendRedirectUrl = ({requestedHost, requestedUrl, queryParameters, secure}) => {
     const siteUrl = urlUtils.urlFor('home', true);
 
-    debug('getsiteRedirectUrl', requestedHost, requestedUrl, siteUrl);
+    debug('getFrontendRedirectUrl', requestedHost, requestedUrl, siteUrl);
 
     // CASE: configured canonical url is HTTPS, but request is HTTP, redirect to requested host + SSL
     if (urlUtils.isSSL(siteUrl) && !secure) {
@@ -89,7 +89,7 @@ _private.getFrontendRedirectUrl = ({requestedHost, requestedUrl, queryParameters
 
 _private.redirect = (req, res, next, redirectFn) => {
     const redirectUrl = redirectFn({
-        requestedHost: req.hostname,
+        requestedHost: req.vhost ? req.vhost.host : req.get('host'),
         requestedUrl: url.parse(req.originalUrl || req.url).pathname,
         queryParameters: req.query,
         secure: req.secure
diff --git a/test/unit/web/middleware/url-redirects_spec.js b/test/unit/web/middleware/url-redirects_spec.js
index 31c803c357..d0d9f0d899 100644
--- a/test/unit/web/middleware/url-redirects_spec.js
+++ b/test/unit/web/middleware/url-redirects_spec.js
@@ -13,8 +13,8 @@ describe('UNIT: url redirects', function () {
 
     beforeEach(function () {
         req = {
-            get hostname() {
-                return host;
+            get vhost() {
+                return {host: host};
             }
         };
         res = {
@@ -356,6 +356,25 @@ describe('UNIT: url redirects', function () {
                 next.called.should.be.true();
                 done();
             });
+
+            it('url and admin url are different, request matches, uses a port', function (done) {
+                urlRedirects.__set__('urlUtils', urlUtils.getInstance({
+                    url: 'https://default.com:2368',
+                    adminUrl: 'https://admin.default.com:2368'
+                }));
+
+                host = 'admin.default.com:2368';
+
+                req.secure = true;
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+
+                res.redirect.called.should.be.false();
+                res.set.called.should.be.false();
+                next.called.should.be.true();
+
+                done();
+            });
         });
     });
 });