Skip to content

Commit a013840

Browse files
gimelfarbgimelfarb
committed
Support for urlSSL config option and forceAdminSSL 403 response
closes #1838 - adding `forceAdminSSL: {redirect: true/false}` option to allow 403 over non-SSL rather than redirect - adding `urlSSL` option to specify SSL variant of `url` - using `urlSSL` when redirecting to SSL (forceAdminSSL), if specified - dynamically patching `.url` property for view engine templates to use SSL variant over HTTPS connections (pass `.secure` property as view engine data) - using `urlSSL` in a "reset password" email, if specified - adding unit tests to test `forceAdminSSL` and `urlSSL` options - created a unit test utility function to dynamically fork a new instance of Ghost during the test, with different configuration options
1 parent 33884e7 commit a013840

File tree

8 files changed

+312
-12
lines changed

8 files changed

+312
-12
lines changed

‎core/server/config/url.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,19 @@ function setConfig(config) {
2626
// Parameters:
2727
// - urlPath - string which must start and end with a slash
2828
// - absolute (optional, default:false) - boolean whether or not the url should be absolute
29+
// - secure (optional, default:false) - boolean whether or not to use urlSSL or url config
2930
// Returns:
3031
// - a URL which always ends with a slash
31-
function createUrl(urlPath, absolute) {
32+
function createUrl(urlPath, absolute, secure) {
3233
urlPath = urlPath || '/';
3334
absolute = absolute || false;
3435

35-
var output = '';
36+
var output = '', baseUrl;
3637

3738
// create base of url, always ends without a slash
3839
if (absolute) {
39-
output += ghostConfig.url.replace(/\/$/, '');
40+
baseUrl = (secure && ghostConfig.urlSSL) ? ghostConfig.urlSSL : ghostConfig.url;
41+
output += baseUrl.replace(/\/$/, '');
4042
} else {
4143
output += ghostConfig.paths.subdir;
4244
}
@@ -99,6 +101,7 @@ function urlPathForPost(post, permalinks) {
99101
// This is probably not the right place for this, but it's the best place for now
100102
function urlFor(context, data, absolute) {
101103
var urlPath = '/',
104+
secure,
102105
knownObjects = ['post', 'tag', 'user'],
103106
knownPaths = {'home': '/', 'rss': '/rss/'}; // this will become really big
104107

@@ -108,22 +111,27 @@ function urlFor(context, data, absolute) {
108111
data = null;
109112
}
110113

114+
// Can pass 'secure' flag in either context or data arg
115+
secure = (context && context.secure) || (data && data.secure);
116+
111117
if (_.isObject(context) && context.relativeUrl) {
112118
urlPath = context.relativeUrl;
113119
} else if (_.isString(context) && _.indexOf(knownObjects, context) !== -1) {
114120
// trying to create a url for an object
115121
if (context === 'post' && data.post && data.permalinks) {
116122
urlPath = urlPathForPost(data.post, data.permalinks);
123+
secure = data.post.secure;
117124
} else if (context === 'tag' && data.tag) {
118125
urlPath = '/tag/' + data.tag.slug + '/';
126+
secure = data.tag.secure;
119127
}
120128
// other objects are recognised but not yet supported
121129
} else if (_.isString(context) && _.indexOf(_.keys(knownPaths), context) !== -1) {
122130
// trying to create a url for a named path
123131
urlPath = knownPaths[context] || '/';
124132
}
125133

126-
return createUrl(urlPath, absolute);
134+
return createUrl(urlPath, absolute, secure);
127135
}
128136

129137
// ## urlForPost

‎core/server/controllers/admin.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,9 @@ adminControllers = {
301301
var email = req.body.email;
302302

303303
api.users.generateResetToken(email).then(function (token) {
304-
var siteLink = '<a href="' + config().url + '">' + config().url + '</a>',
305-
resetUrl = config().url.replace(/\/$/, '') + '/ghost/reset/' + token + '/',
304+
var baseUrl = config().forceAdminSSL ? (config().urlSSL || config().url) : config().url,
305+
siteLink = '<a href="' + baseUrl + '">' + baseUrl + '</a>',
306+
resetUrl = baseUrl.replace(/\/$/, '') + '/ghost/reset/' + token + '/',
306307
resetLink = '<a href="' + resetUrl + '">' + resetUrl + '</a>',
307308
message = {
308309
to: email,

‎core/server/controllers/frontend.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ function handleError(next) {
5656
};
5757
}
5858

59+
// Add Request context parameter to the data object
60+
// to be passed down to the templates
61+
function setReqCtx(req, data) {
62+
(Array.isArray(data) ? data : [data]).forEach(function (d) {
63+
d.secure = req.secure;
64+
});
65+
}
66+
5967
frontendControllers = {
6068
'homepage': function (req, res, next) {
6169
// Parse the page number
@@ -76,6 +84,8 @@ frontendControllers = {
7684
return res.redirect(page.meta.pagination.pages === 1 ? config().paths.subdir + '/' : (config().paths.subdir + '/page/' + page.meta.pagination.pages + '/'));
7785
}
7886

87+
setReqCtx(req, page.posts);
88+
7989
// Render the page of posts
8090
filters.doFilter('prePostsRender', page.posts).then(function (posts) {
8191
res.render('index', formatPageResponse(posts, page));
@@ -113,6 +123,9 @@ frontendControllers = {
113123
return res.redirect(tagUrl(options.tag, page.meta.pagination.pages));
114124
}
115125

126+
setReqCtx(req, page.posts);
127+
setReqCtx(req, page.aspect.tag);
128+
116129
// Render the page of posts
117130
filters.doFilter('prePostsRender', page.posts).then(function (posts) {
118131
api.settings.read('activeTheme').then(function (activeTheme) {
@@ -184,6 +197,9 @@ frontendControllers = {
184197
// Use throw 'no match' to show 404.
185198
throw new Error('no match');
186199
}
200+
201+
setReqCtx(req, post);
202+
187203
filters.doFilter('prePostsRender', post).then(function (post) {
188204
api.settings.read('activeTheme').then(function (activeTheme) {
189205
var paths = config().paths.availableThemes[activeTheme.value],
@@ -279,8 +295,8 @@ frontendControllers = {
279295
var title = result[0].value.value,
280296
description = result[1].value.value,
281297
permalinks = result[2].value,
282-
siteUrl = config.urlFor('home', null, true),
283-
feedUrl = config.urlFor('rss', null, true),
298+
siteUrl = config.urlFor('home', {secure: req.secure}, true),
299+
feedUrl = config.urlFor('rss', {secure: req.secure}, true),
284300
maxPage = page.meta.pagination.pages,
285301
feedItems = [],
286302
feed;
@@ -315,6 +331,8 @@ frontendControllers = {
315331
}
316332
}
317333

334+
setReqCtx(req, page.posts);
335+
318336
filters.doFilter('prePostsRender', page.posts).then(function (posts) {
319337
posts.forEach(function (post) {
320338
var deferred = when.defer(),

‎core/server/middleware/index.js

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,35 @@ function ghostLocals(req, res, next) {
7070
}
7171
}
7272

73+
function initThemeData(secure) {
74+
var themeConfig = config.theme();
75+
if (secure && config().urlSSL) {
76+
// For secure requests override .url property with the SSL version
77+
themeConfig = _.clone(themeConfig);
78+
themeConfig.url = config().urlSSL.replace(/\/$/, '');
79+
}
80+
return themeConfig;
81+
}
82+
7383
// ### InitViews Middleware
7484
// Initialise Theme or Admin Views
7585
function initViews(req, res, next) {
7686
/*jslint unparam:true*/
7787

7888
if (!res.isAdmin) {
79-
hbs.updateTemplateOptions({ data: {blog: config.theme()} });
89+
var themeData = initThemeData(req.secure);
90+
hbs.updateTemplateOptions({ data: {blog: themeData} });
8091
expressServer.engine('hbs', expressServer.get('theme view engine'));
8192
expressServer.set('views', path.join(config().paths.themePath, expressServer.get('activeTheme')));
8293
} else {
8394
expressServer.engine('hbs', expressServer.get('admin view engine'));
8495
expressServer.set('views', config().paths.adminViews);
8596
}
8697

98+
// Pass 'secure' flag to the view engine
99+
// so that templates can choose 'url' vs 'urlSSL'
100+
res.locals.secure = req.secure;
101+
87102
next();
88103
}
89104

@@ -184,9 +199,20 @@ function isSSLrequired(isAdmin) {
184199
function checkSSL(req, res, next) {
185200
if (isSSLrequired(res.isAdmin)) {
186201
if (!req.secure) {
202+
var forceAdminSSL = config().forceAdminSSL,
203+
redirectUrl;
204+
205+
// Check if forceAdminSSL: { redirect: false } is set, which means
206+
// we should just deny non-SSL access rather than redirect
207+
if (forceAdminSSL && forceAdminSSL.redirect !== undefined && !forceAdminSSL.redirect) {
208+
return res.send(403);
209+
}
210+
211+
redirectUrl = url.parse(config().urlSSL || config().url);
187212
return res.redirect(301, url.format({
188213
protocol: 'https:',
189-
hostname: url.parse(config().url).hostname,
214+
hostname: redirectUrl.hostname,
215+
port: redirectUrl.port,
190216
pathname: req.path,
191217
query: req.query
192218
}));

‎core/test/functional/routes/admin_test.js

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,80 @@ describe('Admin Routing', function () {
111111
.end(doEndNoAuth(done));
112112
});
113113
});
114+
115+
// we'll use X-Forwarded-Proto: https to simulate an 'https://' request behind a proxy
116+
describe('Require HTTPS - no redirect', function() {
117+
var forkedGhost, request;
118+
before(function (done) {
119+
var configTestHttps = testUtils.fork.config();
120+
configTestHttps.forceAdminSSL = {redirect: false};
121+
configTestHttps.urlSSL = 'https://localhost/';
122+
123+
testUtils.fork.ghost(configTestHttps, 'testhttps')
124+
.then(function(child) {
125+
forkedGhost = child;
126+
request = require('supertest');
127+
request = request(configTestHttps.url.replace(/\/$/, ''));
128+
}, done)
129+
.then(done);
130+
});
131+
132+
after(function (done) {
133+
if (forkedGhost) {
134+
forkedGhost.kill(done);
135+
}
136+
});
137+
138+
it('should block admin access over non-HTTPS', function(done) {
139+
request.get('/ghost/')
140+
.expect(403)
141+
.end(done);
142+
});
143+
144+
it('should allow admin access over HTTPS', function(done) {
145+
request.get('/ghost/signup/')
146+
.set('X-Forwarded-Proto', 'https')
147+
.expect(200)
148+
.end(doEnd(done));
149+
});
150+
});
151+
152+
describe('Require HTTPS - redirect', function() {
153+
var forkedGhost, request;
154+
before(function (done) {
155+
var configTestHttps = testUtils.fork.config();
156+
configTestHttps.forceAdminSSL = {redirect: true};
157+
configTestHttps.urlSSL = 'https://localhost/';
158+
159+
testUtils.fork.ghost(configTestHttps, 'testhttps')
160+
.then(function(child) {
161+
forkedGhost = child;
162+
request = require('supertest');
163+
request = request(configTestHttps.url.replace(/\/$/, ''));
164+
}, done)
165+
.then(done);
166+
});
167+
168+
after(function (done) {
169+
if (forkedGhost) {
170+
forkedGhost.kill(done);
171+
}
172+
});
173+
174+
it('should redirect admin access over non-HTTPS', function(done) {
175+
request.get('/ghost/')
176+
.expect('Location', /^https:\/\/localhost\/ghost\//)
177+
.expect(301)
178+
.end(done);
179+
});
180+
181+
it('should allow admin access over HTTPS', function(done) {
182+
request.get('/ghost/signup/')
183+
.set('X-Forwarded-Proto', 'https')
184+
.expect(200)
185+
.end(done);
186+
});
187+
});
114188

115189
describe('Ghost Admin Signup', function () {
116190
it('should have a session cookie which expires in 12 hours', function (done) {

‎core/test/functional/routes/frontend_test.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ var request = require('supertest'),
99
express = require('express'),
1010
should = require('should'),
1111
moment = require('moment'),
12+
path = require('path'),
1213

1314
testUtils = require('../../utils'),
1415
ghost = require('../../../../core'),
@@ -142,6 +143,47 @@ describe('Frontend Routing', function () {
142143
.end(doEnd(done));
143144
});
144145
});
146+
147+
// we'll use X-Forwarded-Proto: https to simulate an 'https://' request behind a proxy
148+
describe('HTTPS', function() {
149+
var forkedGhost, request;
150+
before(function (done) {
151+
var configTestHttps = testUtils.fork.config();
152+
configTestHttps.forceAdminSSL = {redirect: false};
153+
configTestHttps.urlSSL = 'https://localhost/';
154+
155+
testUtils.fork.ghost(configTestHttps, 'testhttps')
156+
.then(function(child) {
157+
forkedGhost = child;
158+
request = require('supertest');
159+
request = request(configTestHttps.url.replace(/\/$/, ''));
160+
}, done)
161+
.then(done);
162+
});
163+
164+
after(function (done) {
165+
if (forkedGhost) {
166+
forkedGhost.kill(done);
167+
}
168+
});
169+
170+
it('should set links to url over non-HTTPS', function(done) {
171+
request.get('/')
172+
.expect(200)
173+
.expect(/\<link rel="canonical" href="http:\/\/127.0.0.1:2370\/" \/\>/)
174+
.expect(/copyright \<a href="http:\/\/127.0.0.1:2370\/">Ghost\<\/a\>/)
175+
.end(doEnd(done));
176+
});
177+
178+
it('should set links to urlSSL over HTTPS', function(done) {
179+
request.get('/')
180+
.set('X-Forwarded-Proto', 'https')
181+
.expect(200)
182+
.expect(/\<link rel="canonical" href="https:\/\/localhost\/" \/\>/)
183+
.expect(/copyright \<a href="https:\/\/localhost\/">Ghost\<\/a\>/)
184+
.end(doEnd(done));
185+
});
186+
});
145187

146188
describe('RSS', function () {
147189
it('should redirect without slash', function (done) {

0 commit comments

Comments
 (0)