Skip to content

Commit

Permalink
Redirect feed -> rss
Browse files Browse the repository at this point in the history
closes TryGhost#2261

- reserved 'feed' in the list of reserved keywords for slugs
- added a 301 redirect from /feed/ to /rss/
- added a route test, and realised that standard express redirects don't get the right headers
- fixed the headers across all 301 redirects & added tests for the admin redirects
- removed the redirect from /ghost/login/ to /ghost/signin/ as this happens automatically if you're logged out, and isn't very useful if you're logged in as it just redirects again to /ghost/
  • Loading branch information
ErisDS committed Mar 24, 2014
1 parent ae3c367 commit 13b65ce
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 9 deletions.
2 changes: 1 addition & 1 deletion core/server/models/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
slug = slug.charAt(slug.length - 1) === '-' ? slug.substr(0, slug.length - 1) : slug;

// Check the filtered slug doesn't match any of the reserved keywords
slug = /^(ghost|ghost\-admin|admin|wp\-admin|wp\-login|dashboard|logout|login|signin|signup|signout|register|archive|archives|category|categories|tag|tags|page|pages|post|posts|user|users|rss)$/g
slug = /^(ghost|ghost\-admin|admin|wp\-admin|wp\-login|dashboard|logout|login|signin|signup|signout|register|archive|archives|category|categories|tag|tags|page|pages|post|posts|user|users|rss|feed)$/g
.test(slug) ? slug + '-' + baseName : slug;

//if slug is empty after trimming use "post"
Expand Down
13 changes: 8 additions & 5 deletions core/server/routes/admin.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
var admin = require('../controllers/admin'),
config = require('../config'),
middleware = require('../middleware').middleware;
middleware = require('../middleware').middleware,

ONE_HOUR_S = 60 * 60,
ONE_YEAR_S = 365 * 24 * ONE_HOUR_S;

module.exports = function (server) {
var subdir = config().paths.subdir;
// ### Admin routes
server.get('/logout/', function redirect(req, res) {
/*jslint unparam:true*/
res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S});
res.redirect(301, subdir + '/ghost/signout/');
});
server.get('/signout/', function redirect(req, res) {
/*jslint unparam:true*/
res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S});
res.redirect(301, subdir + '/ghost/signout/');
});
server.get('/signin/', function redirect(req, res) {
/*jslint unparam:true*/
res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S});
res.redirect(301, subdir + '/ghost/signin/');
});
server.get('/signup/', function redirect(req, res) {
/*jslint unparam:true*/
res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S});
res.redirect(301, subdir + '/ghost/signup/');
});
server.get('/ghost/login/', function redirect(req, res) {
/*jslint unparam:true*/
res.redirect(301, subdir + '/ghost/signin/');
});

server.get('/ghost/signout/', admin.signout);
server.get('/ghost/signin/', middleware.redirectToSignup, middleware.redirectToDashboard, admin.signin);
Expand Down
17 changes: 15 additions & 2 deletions core/server/routes/frontend.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
var frontend = require('../controllers/frontend');
var frontend = require('../controllers/frontend'),
config = require('../config'),

ONE_HOUR_S = 60 * 60,
ONE_YEAR_S = 365 * 24 * ONE_HOUR_S;

module.exports = function (server) {
/*jslint regexp: true */
var subdir = config().paths.subdir;

// ### Frontend routes
server.get('/rss/', frontend.rss);
server.get('/rss/:page/', frontend.rss);
server.get('/feed/', function redirect(req, res) {
/*jshint unused:true*/
res.set({'Cache-Control': 'public, max-age=' + ONE_YEAR_S});
res.redirect(301, subdir + '/rss/');
});


server.get('/tag/:slug/rss/', frontend.rss);
server.get('/tag/:slug/rss/:page/', frontend.rss);
server.get('/tag/:slug/page/:page/', frontend.tag);
server.get('/tag/:slug/', frontend.tag);
server.get('/page/:page/', frontend.homepage);
server.get('/', frontend.homepage);
server.get('*', frontend.single);


};
2 changes: 1 addition & 1 deletion core/test/functional/admin/login_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ CasperTest.begin("Login limit is in place", 3, function suite(test) {
}, true);

CasperTest.begin("Can login to Ghost", 4, function suite(test) {
casper.thenOpen(url + "ghost/login/", function testTitle() {
casper.thenOpen(url + "ghost/signin/", function testTitle() {
test.assertTitle("Ghost Admin", "Ghost admin has no title");
});

Expand Down
50 changes: 50 additions & 0 deletions core/test/functional/routes/admin_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,21 @@ describe('Admin Routing', function () {
};
}

function doEndNoAuth(done) {
return function (err, res) {
if (err) {
return done(err);
}

should.not.exist(res.headers['x-cache-invalidate']);
should.not.exist(res.headers['X-CSRF-Token']);
should.not.exist(res.headers['set-cookie']);
should.exist(res.headers.date);

done();
};
}

before(function (done) {
testUtils.clearData().then(function () {
// we initialise data, but not a user.
Expand All @@ -49,6 +64,41 @@ describe('Admin Routing', function () {
request = request(config().url);
});

describe('Legacy Redirects', function () {

it('should redirect /logout/ to /ghost/signout/', function (done) {
request.get('/logout/')
.expect('Location', '/ghost/signout/')
.expect('Cache-Control', cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
});

it('should redirect /signout/ to /ghost/signout/', function (done) {
request.get('/signout/')
.expect('Location', '/ghost/signout/')
.expect('Cache-Control', cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
});

it('should redirect /signin/ to /ghost/signin/', function (done) {
request.get('/signin/')
.expect('Location', '/ghost/signin/')
.expect('Cache-Control', cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
});

it('should redirect /signup/ to /ghost/signup/', function (done) {
request.get('/signup/')
.expect('Location', '/ghost/signup/')
.expect('Cache-Control', cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
});
});

describe('Ghost Admin Signup', function () {
it('should have a session cookie which expires in 12 hours', function (done) {
request.get('/ghost/signup/')
Expand Down
8 changes: 8 additions & 0 deletions core/test/functional/routes/frontend_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ describe('Frontend Routing', function () {
.expect(302)
.end(doEnd(done));
});

it('should get redirected to /rss/ from /feed/', function (done) {
request.get('/feed/')
.expect('Location', '/rss/')
.expect('Cache-Control', cacheRules.year)
.expect(301)
.end(doEnd(done));
});
});

// ### The rest of the tests require more data
Expand Down

0 comments on commit 13b65ce

Please sign in to comment.