From 4a6f58d8d148e86bfdfe6241071ae4ce7adafaa3 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 14 Mar 2017 16:03:30 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=99=85=F0=9F=8F=BD=20Admin=20server=20spl?= =?UTF-8?q?it=20(#8142)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #8140 ✨ Support new default-prod.hbs template for admin ✨ Redirect ghost admin urls without a # ✨ Update admin urls to include # 🎨 Move the admin templates 🔥 Remove redirect to setup middleware 🚨 Tests for new middleware --- .gitignore | 2 +- Gruntfile.js | 6 +- core/server/admin/app.js | 4 +- core/server/admin/controller.js | 5 +- core/server/admin/middleware.js | 14 +++ core/server/admin/views/.gitkeep | 0 core/server/blog/routes.js | 4 +- core/server/config/overrides.json | 2 +- .../controllers/frontend/channel-config.js | 4 +- core/server/middleware/redirect-to-setup.js | 19 --- core/test/functional/routes/admin_spec.js | 40 ++----- core/test/functional/routes/channel_spec.js | 4 +- core/test/unit/admin_spec.js | 63 ++++++++++ .../controllers/frontend/channels_spec.js | 2 +- .../unit/middleware/redirect-to-setup_spec.js | 108 ------------------ 15 files changed, 102 insertions(+), 175 deletions(-) create mode 100644 core/server/admin/middleware.js create mode 100644 core/server/admin/views/.gitkeep delete mode 100644 core/server/middleware/redirect-to-setup.js create mode 100644 core/test/unit/admin_spec.js delete mode 100644 core/test/unit/middleware/redirect-to-setup_spec.js diff --git a/.gitignore b/.gitignore index 7f3c2549b1..171741400e 100644 --- a/.gitignore +++ b/.gitignore @@ -65,7 +65,7 @@ config.*.json # Built asset files /core/built -/core/server/views/default.hbs +/core/server/admin/views/default*.hbs /core/shared/ghost-url.min.js # Coverage reports diff --git a/Gruntfile.js b/Gruntfile.js index 780118ffbe..a10d014dab 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -46,7 +46,7 @@ var overrides = require('./core/server/overrides'), pkg: grunt.file.readJSON('package.json'), clientFiles: [ - 'server/views/default.hbs', + 'server/admin/views/default.hbs', 'built/assets/ghost.js', 'built/assets/ghost.css', 'built/assets/vendor.js', @@ -465,7 +465,7 @@ var overrides = require('./core/server/overrides'), return grunt.task.run(['lint']); } - grunt.task.run(['stubClientFiles', 'test-all']); + grunt.task.run(['test-all']); }); // ### Test-All @@ -486,7 +486,7 @@ var overrides = require('./core/server/overrides'), // ### test-setup *(utility)( // `grunt test-setup` will run all the setup tasks required for running tests grunt.registerTask('test-setup', 'Setup ready to run tests', - ['knex-migrator', 'clean:test', 'setTestEnv'] + ['knex-migrator', 'clean:test', 'setTestEnv', 'stubClientFiles'] ); // ### Unit Tests *(sub task)* diff --git a/core/server/admin/app.js b/core/server/admin/app.js index 31ebefc6a3..d8191c5b47 100644 --- a/core/server/admin/app.js +++ b/core/server/admin/app.js @@ -4,7 +4,7 @@ var debug = require('debug')('ghost:admin'), adminHbs = require('express-hbs').create(), // Admin only middleware - redirectToSetup = require('../middleware/redirect-to-setup'), + adminMiddleware = require('./middleware'), // Global/shared middleware? cacheControl = require('../middleware/cache-control'), @@ -61,7 +61,7 @@ module.exports = function setupAdminApp() { // Admin is currently set to not be cached at all adminApp.use(cacheControl('private')); // Special redirects for the admin (these should have their own cache-control headers) - adminApp.use(redirectToSetup); + adminApp.use(adminMiddleware); // Finally, routing adminApp.get('*', require('./controller')); diff --git a/core/server/admin/controller.js b/core/server/admin/controller.js index c5b2614a13..a5b0947819 100644 --- a/core/server/admin/controller.js +++ b/core/server/admin/controller.js @@ -1,5 +1,6 @@ var debug = require('debug')('ghost:admin:controller'), _ = require('lodash'), + config = require('../config'), api = require('../api'), updateCheck = require('../update-check'), logging = require('../logging'), @@ -33,7 +34,9 @@ module.exports = function adminController(req, res) { } }); }).finally(function noMatterWhat() { - res.render('default'); + var defaultTemplate = config.get('env') === 'production' ? 'default-prod' : 'default'; + + res.render(defaultTemplate); }).catch(function (err) { logging.error(err); }); diff --git a/core/server/admin/middleware.js b/core/server/admin/middleware.js new file mode 100644 index 0000000000..8485436c9d --- /dev/null +++ b/core/server/admin/middleware.js @@ -0,0 +1,14 @@ +var utils = require('../utils'); + +function redirectAdminUrls(req, res, next) { + var ghostPathMatch = req.originalUrl.match(/^\/ghost\/(.+)$/); + if (ghostPathMatch) { + return res.redirect(utils.url.urlJoin(utils.url.urlFor('admin'), '#', ghostPathMatch[1])); + } + + next(); +} + +module.exports = [ + redirectAdminUrls +]; diff --git a/core/server/admin/views/.gitkeep b/core/server/admin/views/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/core/server/blog/routes.js b/core/server/blog/routes.js index 4bc918f480..a555fafaed 100644 --- a/core/server/blog/routes.js +++ b/core/server/blog/routes.js @@ -13,10 +13,10 @@ frontendRoutes = function frontendRoutes() { // ### Admin routes router.get(/^\/(logout|signout)\/$/, function redirectToSignout(req, res) { - utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), 'signout/')); + utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), '#/signout/')); }); router.get(/^\/signup\/$/, function redirectToSignup(req, res) { - utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), 'signup/')); + utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), '#/signup/')); }); // redirect to /ghost and let that do the authentication to prevent redirects to /ghost//admin etc. diff --git a/core/server/config/overrides.json b/core/server/config/overrides.json index 0e5b8c58fd..7108c74cf4 100644 --- a/core/server/config/overrides.json +++ b/core/server/config/overrides.json @@ -4,7 +4,7 @@ "corePath": "core/", "clientAssets": "core/built/assets", "helperTemplates": "core/server/helpers/tpl/", - "adminViews": "core/server/views/", + "adminViews": "core/server/admin/views/", "defaultViews": "core/server/views/", "internalAppPath": "core/server/apps/", "internalStoragePath": "core/server/storage/", diff --git a/core/server/controllers/frontend/channel-config.js b/core/server/controllers/frontend/channel-config.js index ed62b29bb1..99725188a6 100644 --- a/core/server/controllers/frontend/channel-config.js +++ b/core/server/controllers/frontend/channel-config.js @@ -24,7 +24,7 @@ channelConfig = function channelConfig() { } }, slugTemplate: true, - editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '/settings/tags/:slug/') + editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '#/settings/tags/:slug/') }, author: { name: 'author', @@ -40,7 +40,7 @@ channelConfig = function channelConfig() { } }, slugTemplate: true, - editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '/team/:slug/') + editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '#/team/:slug/') } }; diff --git a/core/server/middleware/redirect-to-setup.js b/core/server/middleware/redirect-to-setup.js deleted file mode 100644 index 6c220b181b..0000000000 --- a/core/server/middleware/redirect-to-setup.js +++ /dev/null @@ -1,19 +0,0 @@ -var api = require('../api'), - utils = require('../utils'); - -// Redirect to setup if no user exists -function redirectToSetup(req, res, next) { - var isSetupRequest = req.path.match(/\/setup\//), - isOauthAuthorization = req.path.match(/\/$/) && req.query && (req.query.code || req.query.error); - - api.authentication.isSetup().then(function then(exists) { - if (!exists.setup[0].status && !isSetupRequest && !isOauthAuthorization) { - return res.redirect(utils.url.urlJoin(utils.url.urlFor('admin') + 'setup/')); - } - next(); - }).catch(function handleError(err) { - return next(new Error(err)); - }); -} - -module.exports = redirectToSetup; diff --git a/core/test/functional/routes/admin_spec.js b/core/test/functional/routes/admin_spec.js index 9d7eedb630..bb92da50b7 100644 --- a/core/test/functional/routes/admin_spec.js +++ b/core/test/functional/routes/admin_spec.js @@ -82,25 +82,25 @@ describe('Admin Routing', function () { }); describe('Legacy Redirects', function () { - it('should redirect /logout/ to /ghost/signout/', function (done) { + it('should redirect /logout/ to /ghost/#/signout/', function (done) { request.get('/logout/') - .expect('Location', '/ghost/signout/') + .expect('Location', '/ghost/#/signout/') .expect('Cache-Control', testUtils.cacheRules.year) .expect(301) .end(doEndNoAuth(done)); }); - it('should redirect /signout/ to /ghost/signout/', function (done) { + it('should redirect /signout/ to /ghost/#/signout/', function (done) { request.get('/signout/') - .expect('Location', '/ghost/signout/') + .expect('Location', '/ghost/#/signout/') .expect('Cache-Control', testUtils.cacheRules.year) .expect(301) .end(doEndNoAuth(done)); }); - it('should redirect /signup/ to /ghost/signup/', function (done) { + it('should redirect /signup/ to /ghost/#/signup/', function (done) { request.get('/signup/') - .expect('Location', '/ghost/signup/') + .expect('Location', '/ghost/#/signup/') .expect('Cache-Control', testUtils.cacheRules.year) .expect(301) .end(doEndNoAuth(done)); @@ -130,32 +130,6 @@ describe('Admin Routing', function () { .end(doEndNoAuth(done)); }); }); - - describe('Ghost Admin Setup', function () { - it('should redirect from /ghost/ to /ghost/setup/ when no user/not installed yet', function (done) { - request.get('/ghost/') - .expect('Location', /ghost\/setup/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(302) - .end(doEnd(done)); - }); - - it('should redirect from /ghost/signin/ to /ghost/setup/ when no user', function (done) { - request.get('/ghost/signin/') - .expect('Location', /ghost\/setup/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(302) - .end(doEnd(done)); - }); - - it('should respond with html for /ghost/setup/', function (done) { - request.get('/ghost/setup/') - .expect('Content-Type', /html/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(200) - .end(doEnd(done)); - }); - }); }); describe('FORK', function () { @@ -194,7 +168,7 @@ describe('Admin Routing', function () { }); it('should allow admin access over HTTPS', function (done) { - request.get('/ghost/setup/') + request.get('/ghost/') .set('X-Forwarded-Proto', 'https') .expect(200) .end(doEnd(done)); diff --git a/core/test/functional/routes/channel_spec.js b/core/test/functional/routes/channel_spec.js index 05bfa555c0..598787ad36 100644 --- a/core/test/functional/routes/channel_spec.js +++ b/core/test/functional/routes/channel_spec.js @@ -380,7 +380,7 @@ describe('Channel Routes', function () { it('should redirect to tag settings', function (done) { request.get('/tag/getting-started/edit/') - .expect('Location', '/ghost/settings/tags/getting-started/') + .expect('Location', '/ghost/#/settings/tags/getting-started/') .expect('Cache-Control', testUtils.cacheRules.public) .expect(302) .end(doEnd(done)); @@ -549,7 +549,7 @@ describe('Channel Routes', function () { it('should redirect to editor', function (done) { request.get('/author/ghost-owner/edit/') - .expect('Location', '/ghost/team/ghost-owner/') + .expect('Location', '/ghost/#/team/ghost-owner/') .expect('Cache-Control', testUtils.cacheRules.public) .expect(302) .end(doEnd(done)); diff --git a/core/test/unit/admin_spec.js b/core/test/unit/admin_spec.js new file mode 100644 index 0000000000..01ffa727b4 --- /dev/null +++ b/core/test/unit/admin_spec.js @@ -0,0 +1,63 @@ +var should = require('should'), // jshint ignore:line + sinon = require('sinon'), + + // Thing we are testing + redirectAdminUrls = require('../../server/admin/middleware')[0], + + sandbox = sinon.sandbox.create(); +describe('Admin App', function () { + afterEach(function () { + sandbox.restore(); + }); + + describe('middleware', function () { + describe('redirectAdminUrls', function () { + var req, res, next; + // Input: req.originalUrl + // Output: either next or res.redirect are called + beforeEach(function () { + req = {}; + res = {}; + next = sandbox.stub(); + res.redirect = sandbox.stub(); + }); + + it('should redirect a url which starts with ghost', function () { + req.originalUrl = '/ghost/x'; + + redirectAdminUrls(req, res, next); + + next.called.should.be.false(); + res.redirect.called.should.be.true(); + res.redirect.calledWith('/ghost/#/x').should.be.true(); + }); + + it('should not redirect /ghost/ on its owh', function () { + req.originalUrl = '/ghost/'; + + redirectAdminUrls(req, res, next); + + next.called.should.be.true(); + res.redirect.called.should.be.false(); + }); + + it('should not redirect url that has no slash', function () { + req.originalUrl = 'ghost/x'; + + redirectAdminUrls(req, res, next); + + next.called.should.be.true(); + res.redirect.called.should.be.false(); + }); + + it('should not redirect url that starts with something other than /ghost/', function () { + req.originalUrl = 'x/ghost/x'; + + redirectAdminUrls(req, res, next); + + next.called.should.be.true(); + res.redirect.called.should.be.false(); + }); + }); + }); +}); diff --git a/core/test/unit/controllers/frontend/channels_spec.js b/core/test/unit/controllers/frontend/channels_spec.js index f9f7c300c5..e2d57e5d1c 100644 --- a/core/test/unit/controllers/frontend/channels_spec.js +++ b/core/test/unit/controllers/frontend/channels_spec.js @@ -381,7 +381,7 @@ describe('Channels', function () { describe('Edit', function () { it('should redirect /edit/ to ghost admin', function (done) { testChannelRedirect({url: '/tag/my-tag/edit/'}, function (path) { - path.should.eql('/ghost/settings/tags/my-tag/'); + path.should.eql('/ghost/#/settings/tags/my-tag/'); postAPIStub.called.should.be.false(); }, done); }); diff --git a/core/test/unit/middleware/redirect-to-setup_spec.js b/core/test/unit/middleware/redirect-to-setup_spec.js deleted file mode 100644 index 5e5c82587d..0000000000 --- a/core/test/unit/middleware/redirect-to-setup_spec.js +++ /dev/null @@ -1,108 +0,0 @@ -var sinon = require('sinon'), - should = require('should'), - Promise = require('bluebird'), - api = require('../../../server/api'), - redirectToSetup = require('../../../server/middleware/redirect-to-setup'); - -should.equal(true, true); - -describe('redirectToSetup', function () { - var res, req, next, sandbox; - - beforeEach(function () { - sandbox = sinon.sandbox.create(); - - res = sinon.spy(); - req = sinon.spy(); - next = sinon.spy(); - }); - - afterEach(function () { - sandbox.restore(); - }); - - it('should redirect to setup if not on setup', function (done) { - sandbox.stub(api.authentication, 'isSetup', function () { - return Promise.resolve({setup: [{status: false}]}); - }); - - req.path = '/'; - res.redirect = sinon.spy(function () { - next.called.should.be.false(); - res.redirect.called.should.be.true(); - done(); - }); - - redirectToSetup(req, res, next); - }); - - it('should not redirect if setup is done', function (done) { - sandbox.stub(api.authentication, 'isSetup', function () { - return Promise.resolve({setup: [{status: true}]}); - }); - - res = {redirect: sinon.spy()}; - req.path = '/'; - - next = sinon.spy(function () { - next.called.should.be.true(); - res.redirect.called.should.be.false(); - done(); - }); - - redirectToSetup(req, res, next); - }); - - it('should not redirect if already on setup', function (done) { - sandbox.stub(api.authentication, 'isSetup', function () { - return Promise.resolve({setup: [{status: false}]}); - }); - - res = {redirect: sinon.spy()}; - req.path = '/ghost/setup/'; - - next = sinon.spy(function () { - next.called.should.be.true(); - res.redirect.called.should.be.false(); - done(); - }); - - redirectToSetup(req, res, next); - }); - - it('should not redirect successful oauth authorization requests', function (done) { - sandbox.stub(api.authentication, 'isSetup', function () { - return Promise.resolve({setup: [{status: false}]}); - }); - - res = {redirect: sinon.spy()}; - req.path = '/'; - req.query = {code: 'authCode'}; - - next = sinon.spy(function () { - next.called.should.be.true(); - res.redirect.called.should.be.false(); - done(); - }); - - redirectToSetup(req, res, next); - }); - - it('should not redirect failed oauth authorization requests', function (done) { - sandbox.stub(api.authentication, 'isSetup', function () { - return Promise.resolve({setup: [{status: false}]}); - }); - - res = {redirect: sinon.spy()}; - req.path = '/'; - req.query = {error: 'access_denied', state: 'randomstring'}; - - next = sinon.spy(function () { - next.called.should.be.true(); - res.redirect.called.should.be.false(); - done(); - }); - - redirectToSetup(req, res, next); - }); -});