mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-11-28 05:37:34 +03:00
Use destructuring for more readable redirect code
- This code was a little verbose, which made it hard to see what was happening (it still is a bit) - Used destructuring to reduce the code - Renamed a few variables
This commit is contained in:
Hannah Wolfe
parent
19dde146c1
commit
d4cd996e20
@ -5,11 +5,8 @@ const urlUtils = require('../../../lib/url-utils');
|
||||
|
||||
const _private = {};
|
||||
|
||||
_private.redirectUrl = (options) => {
|
||||
const redirectTo = options.redirectTo;
|
||||
const query = options.query;
|
||||
_private.redirectUrl = ({redirectTo, query, pathname}) => {
|
||||
const parts = url.parse(redirectTo);
|
||||
let pathname = options.path;
|
||||
|
||||
// CASE: ensure we always add a trailing slash to reduce the number of redirects
|
||||
// e.g. you are redirected from example.com/ghost to admin.example.com/ghost and Ghost would detect a missing slash and redirect you to /ghost/
|
||||
@ -27,61 +24,42 @@ _private.redirectUrl = (options) => {
|
||||
});
|
||||
};
|
||||
|
||||
_private.getAdminRedirectUrl = (options) => {
|
||||
const blogHostWithProtocol = urlUtils.urlFor('home', true);
|
||||
const adminHostWithProtocol = urlUtils.urlFor('admin', true);
|
||||
const adminHostWithoutProtocol = adminHostWithProtocol.replace(/(^\w+:|^)\/\//, '');
|
||||
const blogHostWithoutProtocol = blogHostWithProtocol.replace(/(^\w+:|^)\/\//, '');
|
||||
const requestedHost = options.requestedHost;
|
||||
const requestedUrl = options.requestedUrl;
|
||||
const queryParameters = options.queryParameters;
|
||||
const secure = options.secure;
|
||||
/**
|
||||
* Takes care of
|
||||
*
|
||||
* 1. required SSL redirects
|
||||
* 2. redirect to the correct admin url
|
||||
*/
|
||||
_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+:|^)\/\//, '');
|
||||
|
||||
debug('getAdminRedirectUrl', requestedHost, requestedUrl, adminHostWithoutProtocol, blogHostWithoutProtocol, urlUtils.urlJoin(blogHostWithoutProtocol, 'ghost/'));
|
||||
debug('getAdminRedirectUrl', requestedHost, requestedUrl, adminUrlWithoutProtocol, siteUrlWithoutProtocol, urlUtils.urlJoin(siteUrlWithoutProtocol, 'ghost/'));
|
||||
|
||||
// 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 (adminHostWithoutProtocol !== urlUtils.urlJoin(blogHostWithoutProtocol, 'ghost/') &&
|
||||
adminHostWithoutProtocol !== urlUtils.urlJoin(requestedHost, urlUtils.getSubdir(), 'ghost/')) {
|
||||
if (adminUrlWithoutProtocol !== urlUtils.urlJoin(siteUrlWithoutProtocol, 'ghost/') &&
|
||||
adminUrlWithoutProtocol !== urlUtils.urlJoin(requestedHost, urlUtils.getSubdir(), 'ghost/')) {
|
||||
debug('redirect because admin host does not match');
|
||||
|
||||
return _private.redirectUrl({
|
||||
redirectTo: adminHostWithProtocol,
|
||||
path: requestedUrl,
|
||||
redirectTo: adminUrl,
|
||||
pathname: requestedUrl,
|
||||
query: queryParameters
|
||||
});
|
||||
}
|
||||
|
||||
// CASE: configured admin url is HTTPS, but request is HTTP
|
||||
if (urlUtils.isSSL(adminHostWithProtocol) && !secure) {
|
||||
if (urlUtils.isSSL(adminUrl) && !secure) {
|
||||
debug('redirect because protocol does not match');
|
||||
|
||||
return _private.redirectUrl({
|
||||
redirectTo: adminHostWithProtocol,
|
||||
path: requestedUrl,
|
||||
query: queryParameters
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
_private.getBlogRedirectUrl = (options) => {
|
||||
const blogHostWithProtocol = urlUtils.urlFor('home', true);
|
||||
const requestedHost = options.requestedHost;
|
||||
const requestedUrl = options.requestedUrl;
|
||||
const queryParameters = options.queryParameters;
|
||||
const secure = options.secure;
|
||||
|
||||
debug('getBlogRedirectUrl', requestedHost, requestedUrl, blogHostWithProtocol);
|
||||
|
||||
// CASE: configured canonical url is HTTPS, but request is HTTP, redirect to requested host + SSL
|
||||
if (urlUtils.isSSL(blogHostWithProtocol) && !secure) {
|
||||
debug('redirect because protocol does not match');
|
||||
|
||||
return _private.redirectUrl({
|
||||
redirectTo: `https://${requestedHost}`,
|
||||
path: requestedUrl,
|
||||
redirectTo: adminUrl,
|
||||
pathname: requestedUrl,
|
||||
query: queryParameters
|
||||
});
|
||||
}
|
||||
@ -91,9 +69,24 @@ _private.getBlogRedirectUrl = (options) => {
|
||||
* Takes care of
|
||||
*
|
||||
* 1. required SSL redirects
|
||||
* 2. redirect to the correct admin url
|
||||
*
|
||||
*/
|
||||
_private.getFrontendRedirectUrl = ({requestedHost, requestedUrl, queryParameters, secure}) => {
|
||||
const siteUrl = urlUtils.urlFor('home', true);
|
||||
|
||||
debug('getsiteRedirectUrl', requestedHost, requestedUrl, siteUrl);
|
||||
|
||||
// CASE: configured canonical url is HTTPS, but request is HTTP, redirect to requested host + SSL
|
||||
if (urlUtils.isSSL(siteUrl) && !secure) {
|
||||
debug('redirect because protocol does not match');
|
||||
|
||||
return _private.redirectUrl({
|
||||
redirectTo: `https://${requestedHost}`,
|
||||
pathname: requestedUrl,
|
||||
query: queryParameters
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
_private.redirect = (req, res, next, redirectFn) => {
|
||||
const redirectUrl = redirectFn({
|
||||
requestedHost: req.hostname,
|
||||
@ -112,7 +105,7 @@ _private.redirect = (req, res, next, redirectFn) => {
|
||||
};
|
||||
|
||||
const frontendRedirect = (req, res, next) => {
|
||||
_private.redirect(req, res, next, _private.getBlogRedirectUrl);
|
||||
_private.redirect(req, res, next, _private.getFrontendRedirectUrl);
|
||||
};
|
||||
|
||||
const adminRedirect = (req, res, next) => {
|
||||
|
||||
@ -1,12 +1,12 @@
|
||||
var should = require('should'),
|
||||
sinon = require('sinon'),
|
||||
rewire = require('rewire'),
|
||||
urlUtils = require('../../../utils/urlUtils'),
|
||||
urlRedirects = rewire('../../../../core/server/web/shared/middlewares/url-redirects'),
|
||||
{frontendSSLRedirect, adminSSLAndHostRedirect} = urlRedirects,
|
||||
getAdminRedirectUrl = urlRedirects.__get__('_private.getAdminRedirectUrl'),
|
||||
getBlogRedirectUrl = urlRedirects.__get__('_private.getBlogRedirectUrl'),
|
||||
redirect = urlRedirects.__get__('_private.redirect');
|
||||
const should = require('should');
|
||||
const sinon = require('sinon');
|
||||
const rewire = require('rewire');
|
||||
const urlUtils = require('../../../utils/urlUtils');
|
||||
const urlRedirects = rewire('../../../../core/server/web/shared/middlewares/url-redirects');
|
||||
const {frontendSSLRedirect, adminSSLAndHostRedirect} = urlRedirects;
|
||||
const getAdminRedirectUrl = urlRedirects.__get__('_private.getAdminRedirectUrl');
|
||||
const getFrontendRedirectUrl = urlRedirects.__get__('_private.getFrontendRedirectUrl');
|
||||
const redirect = urlRedirects.__get__('_private.redirect');
|
||||
|
||||
describe('UNIT: url redirects', function () {
|
||||
var res, req, next, host;
|
||||
@ -38,12 +38,12 @@ describe('UNIT: url redirects', function () {
|
||||
urlRedirects.__set__('_private.redirect', redirectSpy);
|
||||
});
|
||||
|
||||
it('frontendSSLRedirect passes getBlogRedirectUrl', function () {
|
||||
it('frontendSSLRedirect passes getSiteRedirectUrl', function () {
|
||||
urlRedirects.__set__('urlUtils', urlUtils.getInstance({url: 'https://default.com:2368/'}));
|
||||
|
||||
frontendSSLRedirect(req, res, next);
|
||||
|
||||
redirectSpy.calledWith(req, res, next, getBlogRedirectUrl).should.eql(true);
|
||||
redirectSpy.calledWith(req, res, next, getFrontendRedirectUrl).should.eql(true);
|
||||
});
|
||||
|
||||
it('adminSSLAndHostRedirect passes getAdminRedirectUrl', function () {
|
||||
@ -56,7 +56,7 @@ describe('UNIT: url redirects', function () {
|
||||
});
|
||||
|
||||
describe('expect redirect', function () {
|
||||
it('blog is https, request is http', function (done) {
|
||||
it('site is https, request is http', function (done) {
|
||||
urlRedirects.__set__('urlUtils', urlUtils.getInstance({
|
||||
url: 'https://default.com:2368/'
|
||||
}));
|
||||
@ -64,7 +64,7 @@ describe('UNIT: url redirects', function () {
|
||||
host = 'default.com:2368';
|
||||
|
||||
req.originalUrl = '/';
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.false();
|
||||
res.redirect.called.should.be.true();
|
||||
res.redirect.calledWith(301, 'https://default.com:2368/').should.be.true();
|
||||
@ -72,14 +72,14 @@ describe('UNIT: url redirects', function () {
|
||||
done();
|
||||
});
|
||||
|
||||
it('blog host is !== request host', function (done) {
|
||||
it('site host is !== request host', function (done) {
|
||||
urlRedirects.__set__('urlUtils', urlUtils.getInstance({
|
||||
url: 'https://default.com'
|
||||
}));
|
||||
host = 'localhost:2368';
|
||||
|
||||
req.originalUrl = '/';
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.false();
|
||||
res.redirect.called.should.be.true();
|
||||
res.redirect.calledWith(301, 'https://localhost:2368/').should.be.true();
|
||||
@ -210,7 +210,7 @@ describe('UNIT: url redirects', function () {
|
||||
});
|
||||
|
||||
describe('expect no redirect', function () {
|
||||
it('blog is http, request is http', function (done) {
|
||||
it('site is http, request is http', function (done) {
|
||||
urlRedirects.__set__('urlUtils', urlUtils.getInstance({
|
||||
url: 'http://default.com:2368/'
|
||||
}));
|
||||
@ -218,7 +218,7 @@ describe('UNIT: url redirects', function () {
|
||||
host = 'default.com:2368';
|
||||
|
||||
req.originalUrl = '/';
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.true();
|
||||
res.redirect.called.should.be.false();
|
||||
res.set.called.should.be.false();
|
||||
@ -226,7 +226,7 @@ describe('UNIT: url redirects', function () {
|
||||
done();
|
||||
});
|
||||
|
||||
it('blog is http, request is https', function (done) {
|
||||
it('site is http, request is https', function (done) {
|
||||
urlRedirects.__set__('urlUtils', urlUtils.getInstance({
|
||||
url: 'http://default.com:2368/'
|
||||
}));
|
||||
@ -235,7 +235,7 @@ describe('UNIT: url redirects', function () {
|
||||
|
||||
req.originalUrl = '/';
|
||||
req.secure = true;
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.true();
|
||||
res.redirect.called.should.be.false();
|
||||
res.set.called.should.be.false();
|
||||
@ -251,7 +251,7 @@ describe('UNIT: url redirects', function () {
|
||||
|
||||
req.originalUrl = '/';
|
||||
req.secure = true;
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.true();
|
||||
res.redirect.called.should.be.false();
|
||||
res.set.called.should.be.false();
|
||||
@ -267,7 +267,7 @@ describe('UNIT: url redirects', function () {
|
||||
|
||||
req.originalUrl = '/';
|
||||
req.secure = true;
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.true();
|
||||
res.redirect.called.should.be.false();
|
||||
res.set.called.should.be.false();
|
||||
@ -284,7 +284,7 @@ describe('UNIT: url redirects', function () {
|
||||
|
||||
req.originalUrl = '/';
|
||||
req.secure = true;
|
||||
redirect(req, res, next, getBlogRedirectUrl);
|
||||
redirect(req, res, next, getFrontendRedirectUrl);
|
||||
next.called.should.be.true();
|
||||
res.redirect.called.should.be.false();
|
||||
res.set.called.should.be.false();
|
||||
|
||||
Loading…
Reference in New Issue
Block a user