🎨 add urlFor('admin') and increase usage of urlFor helper (#7935)

refs #7488 - to be able to refactor the url configuration in ghost, we need to go step by step making this possible - reduce the usage of forceAdminSSL - add a urlFor('admin') helper, which returns the admin url + path e.g. http://my-blog.com/blog/ghost - increase usage of urlFor helper - do not expose getBaseUrl, use urlFor('home') (home === blog)
TryGhost · Feb 2, 2017 · 16f5d1f · 16f5d1f
1 parent 85c0913
commit 16f5d1f
Show file tree

Hide file tree

Showing 17 changed files with 108 additions and 46 deletions.
diff --git a/core/server/api/app.js b/core/server/api/app.js
@@ -217,7 +217,7 @@ module.exports = function setupApiApp() {
 
     // @TODO finish refactoring this away.
     apiApp.use(function setIsAdmin(req, res, next) {
-        // Api === isAdmin for the purposes of the forceAdminSSL config option.
+        // api === isAdmin
         res.isAdmin = true;
         next();
     });

diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js
@@ -216,8 +216,8 @@ authentication = {
         }
 
         function sendResetNotification(data) {
-            var baseUrl = config.get('forceAdminSSL') ? globalUtils.url.urlFor('home', {secure: true}, true) : globalUtils.url.urlFor('home', true),
-                resetUrl = globalUtils.url.urlJoin(baseUrl, '/ghost/reset/', globalUtils.encodeBase64URLsafe(data.resetToken), '/');
+            var adminUrl = globalUtils.url.urlFor('admin', true),
+                resetUrl = globalUtils.url.urlJoin(adminUrl, 'reset', globalUtils.encodeBase64URLsafe(data.resetToken), '/');
 
             return mail.utils.generateContent({
                 data: {

diff --git a/core/server/api/invites.js b/core/server/api/invites.js
@@ -9,7 +9,6 @@ var _ = require('lodash'),
     utils = require('./utils'),
     errors = require('../errors'),
     logging = require('../logging'),
-    config = require('../config'),
     i18n = require('../i18n'),
     docName = 'invites',
     allowedIncludes = ['created_by', 'updated_by'],
@@ -98,14 +97,14 @@ invites = {
                     return settings.read({key: 'title'});
                 })
                 .then(function (response) {
-                    var baseUrl = config.get('forceAdminSSL') ? globalUtils.url.urlFor('home', {secure: true}, true) : globalUtils.url.urlFor('home', true);
+                    var adminUrl = globalUtils.url.urlFor('admin', true);
 
                     emailData = {
                         blogName: response.settings[0].value,
                         invitedByName: loggedInUser.get('name'),
                         invitedByEmail: loggedInUser.get('email'),
                         // @TODO: resetLink sounds weird
-                        resetLink: globalUtils.url.urlJoin(baseUrl, 'ghost/signup', globalUtils.encodeBase64URLsafe(invite.get('token')), '/')
+                        resetLink: globalUtils.url.urlJoin(adminUrl, 'signup', globalUtils.encodeBase64URLsafe(invite.get('token')), '/')
                     };
 
                     return mail.utils.generateContent({data: emailData, template: 'invite-user'});

diff --git a/core/server/blog/routes.js b/core/server/blog/routes.js
@@ -9,20 +9,19 @@ var express         = require('express'),
 
 frontendRoutes = function frontendRoutes() {
     var router = express.Router(),
-        subdir = utils.url.getSubdir(),
         routeKeywords = config.get('routeKeywords');
 
     // ### Admin routes
     router.get(/^\/(logout|signout)\/$/, function redirectToSignout(req, res) {
-        utils.redirect301(res, utils.url.urlJoin(subdir, '/ghost/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(subdir, '/ghost/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.
     router.get(/^\/((ghost-admin|admin|wp-admin|dashboard|signin|login)\/?)$/, function redirectToAdmin(req, res) {
-        utils.redirect301(res, utils.url.urlJoin(subdir, '/ghost/'));
+        utils.redirect301(res, utils.url.urlFor('admin'));
     });
 
     // Post Live Preview

diff --git 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: '/ghost/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: '/ghost/team/:slug/'
+            editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '/team/:slug/')
         }
     };
 

diff --git a/core/server/controllers/frontend/index.js b/core/server/controllers/frontend/index.js
@@ -76,7 +76,7 @@ frontendControllers = {
 
             // CASE: last param is of url is /edit, redirect to admin
             if (lookup.isEditURL) {
-                return res.redirect(utils.url.urlJoin(utils.url.getSubdir(), '/ghost/editor', post.id, '/'));
+                return res.redirect(utils.url.urlJoin(utils.url.urlFor('admin'), 'editor', post.id, '/'));
             }
 
             // CASE: permalink is not valid anymore, we redirect him permanently to the correct one

diff --git a/core/server/data/meta/amp_url.js b/core/server/data/meta/amp_url.js
@@ -6,7 +6,7 @@ function getAmplUrl(data) {
     var context = data.context ? data.context : null;
 
     if (_.includes(context, 'post') && !_.includes(context, 'amp')) {
-        return utils.url.urlJoin(utils.url.getBaseUrl(false), getUrl(data, false), 'amp/');
+        return utils.url.urlJoin(utils.url.urlFor('home', true), getUrl(data, false), 'amp/');
     }
     return null;
 }

diff --git a/core/server/data/meta/canonical_url.js b/core/server/data/meta/canonical_url.js
@@ -2,12 +2,12 @@ var utils  = require('../../utils'),
     getUrl = require('./url');
 
 function getCanonicalUrl(data) {
-    var url = utils.url.urlJoin(utils.url.getBaseUrl(false),
-        getUrl(data, false));
+    var url = utils.url.urlJoin(utils.url.urlFor('home', true), getUrl(data, false));
 
     if (url.indexOf('/amp/')) {
         url = url.replace(/\/amp\/$/i, '/');
     }
+
     return url;
 }
 

diff --git a/core/server/data/meta/index.js b/core/server/data/meta/index.js
@@ -51,7 +51,7 @@ function getMetaData(data, root) {
 
     metaData.blog.logo = {};
     metaData.blog.logo.url = config.get('theme').logo ?
-        utils.url.urlFor('image', {image: config.get('theme').logo}, true) : utils.url.urlFor({relativeUrl: '/ghost/img/ghosticon.jpg'}, {}, true);
+        utils.url.urlFor('image', {image: config.get('theme').logo}, true) : utils.url.urlJoin(utils.url.urlFor('admin'), 'img/ghosticon.jpg');
 
     // TODO: cleanup these if statements
     if (data.post && data.post.html) {

diff --git a/core/server/data/slack/index.js b/core/server/data/slack/index.js
@@ -5,7 +5,6 @@ var https           = require('https'),
     logging         = require('../../logging'),
     utils           = require('../../utils'),
     events          = require('../../events'),
-    logging          = require('../../logging'),
     api             = require('../../api/settings'),
     i18n            = require('../../i18n'),
     schema          = require('../schema').checks,
@@ -74,7 +73,7 @@ function ping(post) {
             slackData = {
                 text: message,
                 unfurl_links: true,
-                icon_url: utils.url.urlFor({relativeUrl: '/ghost/img/ghosticon.jpg'}, {}, true),
+                icon_url: utils.url.urlJoin(utils.url.urlFor('admin', true), 'img/ghosticon.jpg'),
                 username: 'Ghost'
             };
 

diff --git a/core/server/index.js b/core/server/index.js
@@ -149,8 +149,8 @@ function init(options) {
         auth.init({
             authType: config.get('auth:type'),
             ghostAuthUrl: config.get('auth:url'),
-            redirectUri: utils.url.urlJoin(utils.url.getBaseUrl(), 'ghost', '/'),
-            clientUri: utils.url.urlJoin(utils.url.getBaseUrl(), '/'),
+            redirectUri: utils.url.urlFor('admin', true),
+            clientUri: utils.url.urlFor('home', true),
             clientName: api.settings.getSettingSync('title'),
             clientDescription: api.settings.getSettingSync('description')
         }).then(function (response) {

diff --git a/core/server/mail/utils.js b/core/server/mail/utils.js
@@ -3,7 +3,6 @@ var _ = require('lodash').runInContext(),
     Promise = require('bluebird'),
     path = require('path'),
     htmlToText = require('html-to-text'),
-    config = require('../config'),
     utils = require('../utils'),
     templatesDir = path.resolve(__dirname, '..', 'mail', 'templates');
 
@@ -14,7 +13,7 @@ exports.generateContent = function generateContent(options) {
         data;
 
     defaults = {
-        siteUrl: config.get('forceAdminSSL') ? utils.url.urlFor('home', {secure: true}, true) : utils.url.urlFor('home', true)
+        siteUrl: utils.url.urlFor('home', true)
     };
 
     data = _.defaults(defaults, options.data);

diff --git a/core/server/middleware/redirect-to-setup.js b/core/server/middleware/redirect-to-setup.js
@@ -8,7 +8,7 @@ function redirectToSetup(req, res, next) {
 
     api.authentication.isSetup().then(function then(exists) {
         if (!exists.setup[0].status && !isSetupRequest && !isOauthAuthorization) {
-            return res.redirect(utils.url.getSubdir() + '/ghost/setup/');
+            return res.redirect(utils.url.urlJoin(utils.url.urlFor('admin') + 'setup/'));
         }
         next();
     }).catch(function handleError(err) {

diff --git a/core/server/utils/url.js b/core/server/utils/url.js
@@ -191,6 +191,7 @@ function urlPathForPost(post) {
 // - data (optional) - a json object containing data needed to generate a url
 // - absolute (optional, default:false) - boolean whether or not the url should be absolute
 // This is probably not the right place for this, but it's the best place for now
+// @TODO: rewrite, very hard to read!
 function urlFor(context, data, absolute) {
     var urlPath = '/',
         secure, imagePathRe,
@@ -262,9 +263,21 @@ function urlFor(context, data, absolute) {
                 absolute = true;
             }
         }
-    } else if (context === 'home' && absolute)  {
+    } else if (context === 'home' && absolute) {
         urlPath = getBaseUrl(secure);
         // other objects are recognised but not yet supported
+    } else if (context === 'admin') {
+        if (config.get('forceAdminSSL')) {
+            urlPath = getBaseUrl(true);
+        } else {
+            urlPath = getBaseUrl();
+        }
+
+        if (absolute) {
+            urlPath += 'ghost/';
+        } else {
+            urlPath = '/ghost/';
+        }
     } else if (_.isString(context) && _.indexOf(_.keys(knownPaths), context) !== -1) {
         // trying to create a url for a named path
         urlPath = knownPaths[context] || '/';
@@ -314,4 +327,3 @@ module.exports.urlJoin = urlJoin;
 module.exports.urlFor = urlFor;
 module.exports.urlPathForPost = urlPathForPost;
 module.exports.apiUrl = apiUrl;
-module.exports.getBaseUrl = getBaseUrl;
diff --git a/core/test/unit/auth/passport_spec.js b/core/test/unit/auth/passport_spec.js
@@ -106,14 +106,14 @@ describe('Ghost Passport', function () {
             client = new models.Client(testUtils.DataGenerator.forKnex.createClient({
                 name: 'Ghost',
                 client_uri: 'http://my-blog.com',
-                redirection_uri: utils.url.getBaseUrl()
+                redirection_uri: utils.url.urlFor('home', true)
             }));
 
             return GhostPassport.init({
                 authType: 'ghost',
                 clientUri: 'http://my-blog.com',
                 ghostAuthUrl: 'http://devauth.ghost.org',
-                redirectUri: utils.url.getBaseUrl(),
+                redirectUri: utils.url.urlFor('home', true),
                 clientName: 'Ghost'
             }).then(function (response) {
                 should.exist(response.passport);
@@ -138,7 +138,7 @@ describe('Ghost Passport', function () {
                 authType: 'ghost',
                 clientUri: 'http://my-blog.com',
                 ghostAuthUrl: 'http://devauth.ghost.org',
-                redirectUri: utils.url.getBaseUrl(),
+                redirectUri: utils.url.urlFor('home', true),
                 clientName: 'Ghost'
             }).then(function (response) {
                 should.exist(response.passport);
@@ -159,7 +159,7 @@ describe('Ghost Passport', function () {
                 authType: 'ghost',
                 clientUri: 'http://my-blog.com',
                 ghostAuthUrl: 'http://devauth.ghost.org',
-                redirectUri: utils.url.getBaseUrl(),
+                redirectUri: utils.url.urlFor('home', true),
                 clientName: 'Ghost'
             }).then(function () {
                 FakeGhostOAuth2Strategy.prototype.registerClient.called.should.eql(true);
@@ -192,7 +192,7 @@ describe('Ghost Passport', function () {
                 authType: 'ghost',
                 clientUri: 'http://my-blog.com',
                 ghostAuthUrl: 'http://devauth.ghost.org',
-                redirectUri: utils.url.getBaseUrl(),
+                redirectUri: utils.url.urlFor('home', true),
                 clientName: 'custom client name'
             }).then(function (response) {
                 should.exist(response.passport);
@@ -220,7 +220,7 @@ describe('Ghost Passport', function () {
                 authType: 'ghost',
                 clientUri: 'http://my-blog.com',
                 ghostAuthUrl: 'http://devauth.ghost.org',
-                redirectUri: utils.url.getBaseUrl()
+                redirectUri: utils.url.urlFor('home', true)
             }).catch(function (err) {
                 (err instanceof errors.GhostError).should.eql(true);
                 FakeGhostOAuth2Strategy.prototype.registerClient.callCount.should.eql(1);

diff --git a/core/test/unit/slack_spec.js b/core/test/unit/slack_spec.js
@@ -8,6 +8,7 @@ var _               = require('lodash'),
     url             = require('url'),
 
     // Stuff we test
+    configUtils    = require('../utils/configUtils'),
     slack          = rewire('../../server/data/slack'),
     events         = require('../../server/events'),
     api            = require('../../server/api/settings'),
@@ -53,6 +54,7 @@ describe('Slack', function () {
 
     afterEach(function () {
         sandbox.restore();
+        configUtils.restore();
     });
 
     it('listen() should initialise event correctly', function () {
@@ -141,7 +143,7 @@ describe('Slack', function () {
     describe('ping()', function () {
         var makeRequestAssertions,
             isPostStub,
-            urlForStub,
+            urlForSpy,
             settingsAPIStub,
             settingsObj,
             slackReset,
@@ -151,36 +153,36 @@ describe('Slack', function () {
 
         beforeEach(function () {
             isPostStub = sandbox.stub(schema, 'isPost');
-            urlForStub = sandbox.stub(utils.url, 'urlFor');
-            urlForStub.withArgs('post').returns('http://myblog.com/post');
-            urlForStub.returns('http://myblog.com/someImageurl.jpg');
+            urlForSpy = sandbox.spy(utils.url, 'urlFor');
             settingsObj = {settings: [], meta: {}};
             settingsAPIStub = sandbox.stub(api, 'read').returns(Promise.resolve(settingsObj));
 
             makeRequestMock = function () {
                 makeRequestAssertions.apply(this, arguments);
             };
+
             makeRequestSpy = sandbox.spy(makeRequestMock);
             slackReset = slack.__set__('makeRequest', makeRequestMock);
+
+            configUtils.set('url', 'http://myblog.com');
         });
 
         afterEach(function () {
             slackReset();
         });
 
         it('makes a request for a post if url is provided', function (done) {
-            // set up
             isPostStub.returns(true);
             settingsObj.settings[0] = slackObjWithUrl;
 
             // assertions
             makeRequestAssertions = function (requestOptions, requestData) {
                 isPostStub.calledOnce.should.be.true();
-                urlForStub.calledTwice.should.be.true();
+                urlForSpy.calledTwice.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
                 requestOptions.should.have.property('href').and.be.equal('https://hooks.slack.com/services/a-b-c-d');
-                requestData.should.have.property('text').and.be.equal('http://myblog.com/post');
-                requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/someImageurl.jpg');
+                requestData.should.have.property('text').and.be.equal('http://myblog.com/');
+                requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/ghost/img/ghosticon.jpg');
                 requestData.should.have.property('username').and.be.equal('Ghost');
                 done();
             };
@@ -193,14 +195,16 @@ describe('Slack', function () {
             isPostStub.returns(false);
             settingsObj.settings[0] = slackObjWithUrl;
 
+            configUtils.set('forceAdminSSL', true);
+
             // assertions
             makeRequestAssertions = function (requestOptions, requestData) {
                 isPostStub.calledOnce.should.be.true();
-                urlForStub.calledOnce.should.be.true();
+                urlForSpy.calledOnce.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
                 requestOptions.should.have.property('href').and.be.equal('https://hooks.slack.com/services/a-b-c-d');
                 requestData.should.have.property('text').and.be.equal('Hi!');
-                requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/someImageurl.jpg');
+                requestData.should.have.property('icon_url').and.be.equal('https://myblog.com/ghost/img/ghosticon.jpg');
                 requestData.should.have.property('username').and.be.equal('Ghost');
                 done();
             };
@@ -217,7 +221,7 @@ describe('Slack', function () {
             ping({page: true}).then(function (result) {
                 // assertions
                 isPostStub.calledOnce.should.be.true();
-                urlForStub.calledOnce.should.be.true();
+                urlForSpy.calledOnce.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
                 makeRequestSpy.called.should.be.false();
                 should.not.exist(result);
@@ -234,7 +238,7 @@ describe('Slack', function () {
             ping({}).then(function (result) {
                 // assertions
                 isPostStub.calledOnce.should.be.true();
-                urlForStub.calledOnce.should.be.true();
+                urlForSpy.calledOnce.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
                 makeRequestSpy.called.should.be.false();
                 should.not.exist(result);
@@ -251,7 +255,7 @@ describe('Slack', function () {
             ping({slug: 'welcome-to-ghost'}).then(function (result) {
                 // assertions
                 isPostStub.calledOnce.should.be.true();
-                urlForStub.calledOnce.should.be.true();
+                urlForSpy.calledOnce.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
                 makeRequestSpy.called.should.be.false();
                 should.not.exist(result);
@@ -266,7 +270,7 @@ describe('Slack', function () {
                 done('This should not get called');
             }).catch(function () {
                 isPostStub.calledOnce.should.be.true();
-                urlForStub.calledOnce.should.be.false();
+                urlForSpy.calledOnce.should.be.false();
                 settingsAPIStub.calledOnce.should.be.true();
                 makeRequestSpy.called.should.be.false();
                 done();