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
This commit is contained in:
Hannah Wolfe 2020-04-23 19:00:28 +01:00 committed by GitHub
parent 43dd253c12
commit 4a67ea5546
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 9 deletions

View File

@ -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

View File

@ -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();
});
});
});
});