Fix urlFor to handle secure correctly

issue #6270
- Exposed getBaseUrl on the config class.
- Fix formatting config index as array was more then 140 characters long.
- Updated getBaseUrl to handle secure by replacing http with https if true.
- Fixed ghost_head helper to output canonical base url no https.
- Fixed ghost_head helper to set secure correctly for the rss link.
- Fixed navigation helper to pass secure in each nav item, so that urlFor can u$
- Fixed {{url}} to pass secure correctly to config.urlFor.
- Fixed test to use urlSSL over https besides for canonical.
- Add tests for {{url}} and to make sure they output https for absolute and secure.
- Update twitter and og url to use the canonical url.

core/server/config/index.js

@@ -33,6 +33,7 @@ function ConfigManager(config) {
     this.urlFor = configUrl.urlFor;
     this.urlPathForPost = configUrl.urlPathForPost;
     this.apiUrl = configUrl.apiUrl;
+    this.getBaseUrl = configUrl.getBaseUrl;
 
     // If we're given an initial config object then we can set it.
     if (config && _.isObject(config)) {
@@ -214,7 +215,10 @@ ConfigManager.prototype.set = function (config) {
             // Used by generateSlug to generate slugs for posts, tags, users, ..
             // reserved slugs are reserved but can be extended/removed by apps
             // protected slugs cannot be changed or removed
-            reserved: ['admin', 'app', 'apps', 'archive', 'archives', 'categories', 'category', 'dashboard', 'feed', 'ghost-admin', 'login', 'logout', 'page', 'pages', 'post', 'posts', 'public', 'register', 'setup', 'signin', 'signout', 'signup', 'user', 'users', 'wp-admin', 'wp-login'],
+            reserved: ['admin', 'app', 'apps', 'archive', 'archives', 'categories',
+            'category', 'dashboard', 'feed', 'ghost-admin', 'login', 'logout',
+            'page', 'pages', 'post', 'posts', 'public', 'register', 'setup',
+            'signin', 'signout', 'signup', 'user', 'users', 'wp-admin', 'wp-login'],
             protected: ['ghost', 'rss']
         },
         uploads: {

core/server/config/url.js

@@ -18,7 +18,15 @@ function setConfig(config) {
 }
 
 function getBaseUrl(secure) {
-    return (secure && ghostConfig.urlSSL) ? ghostConfig.urlSSL : ghostConfig.url;
+    if (secure && ghostConfig.urlSSL) {
+        return ghostConfig.urlSSL;
+    } else {
+        if (secure) {
+            return ghostConfig.url.replace('http://', 'https://');
+        } else {
+            return ghostConfig.url;
+        }
+    }
 }
 
 function urlJoin() {
@@ -190,6 +198,7 @@ function urlFor(context, data, absolute) {
             return urlPath;
         } else if (context === 'nav' && data.nav) {
             urlPath = data.nav.url;
+            secure = data.nav.secure || secure;
             baseUrl = getBaseUrl(secure);
             hostname = baseUrl.split('//')[1] + ghostConfig.paths.subdir;
             if (urlPath.indexOf(hostname) > -1
@@ -242,3 +251,4 @@ module.exports.urlJoin = urlJoin;
 module.exports.urlFor = urlFor;
 module.exports.urlPathForPost = urlPathForPost;
 module.exports.apiUrl = apiUrl;
+module.exports.getBaseUrl = getBaseUrl;

core/server/helpers/ghost_head.js

@@ -109,6 +109,7 @@ function addContextMetaData(context, data, metaData) {
 function initMetaData(context, data, results) {
     var metaData = {
         url: results.url,
+        canonicalUrl: results.canonicalUrl,
         metaDescription: results.meta_description || null,
         metaTitle: results.meta_title,
         coverImage:  results.image,
@@ -148,7 +149,7 @@ function getStructuredData(metaData) {
         'og:type': metaData.ogType,
         'og:title': metaData.metaTitle,
         'og:description': metaData.metaDescription,
-        'og:url': metaData.url,
+        'og:url': metaData.canonicalUrl,
         'og:image': metaData.coverImage,
         'article:published_time': metaData.publishedDate,
         'article:modified_time': metaData.modifiedDate,
@@ -156,7 +157,7 @@ function getStructuredData(metaData) {
         'twitter:card': metaData.card,
         'twitter:title': metaData.metaTitle,
         'twitter:description': metaData.metaDescription,
-        'twitter:url': metaData.url,
+        'twitter:url': metaData.canonicalUrl,
         'twitter:image:src': metaData.coverImage
     };
 
@@ -311,6 +312,8 @@ ghost_head = function (options) {
 
     // Store Async calls in an object of named promises
     props.url = urlHelper.call(self, {hash: {absolute: true}});
+    props.canonicalUrl = config.urlJoin(config.getBaseUrl(false),
+        urlHelper.call(self, {hash: {absolute: false}}));
     props.meta_description = meta_description.call(self, options);
     props.meta_title = meta_title.call(self, options);
     props.client = getClient();
@@ -330,7 +333,7 @@ ghost_head = function (options) {
             }
 
             // head is our main array that holds our meta data
-            head.push('<link rel="canonical" href="' + metaData.url + '" />');
+            head.push('<link rel="canonical" href="' + metaData.canonicalUrl + '" />');
             head.push('<meta name="referrer" content="origin" />');
 
             // Generate context driven pagination urls
@@ -359,7 +362,8 @@ ghost_head = function (options) {
 
         head.push('<meta name="generator" content="Ghost ' + safeVersion + '" />');
         head.push('<link rel="alternate" type="application/rss+xml" title="' +
-            title  + '" href="' + config.urlFor('rss', null, true) + '" />');
+            title  + '" href="' + config.urlFor('rss', {secure: self.secure},
+            true) + '" />');
     }).then(function () {
         return api.settings.read({key: 'ghost_head'});
     }).then(function (response) {

core/server/helpers/navigation.js

@@ -13,6 +13,7 @@ navigation = function (options) {
     /*jshint unused:false*/
     var navigationData = options.data.blog.navigation,
         currentUrl = options.data.root.relativeUrl,
+        self = this,
         output,
         context;
 
@@ -49,6 +50,7 @@ navigation = function (options) {
         out.label = e.label;
         out.slug = _slugify(e.label);
         out.url = hbs.handlebars.Utils.escapeExpression(e.url);
+        out.secure = self.secure;
         return out;
     });
 

core/server/helpers/url.js

@@ -12,22 +12,22 @@ url = function (options) {
     var absolute = options && options.hash.absolute;
 
     if (schema.isPost(this)) {
-        return config.urlFor('post', {post: this}, absolute);
+        return config.urlFor('post', {post: this, secure: this.secure}, absolute);
     }
 
     if (schema.isTag(this)) {
-        return config.urlFor('tag', {tag: this}, absolute);
+        return config.urlFor('tag', {tag: this, secure: this.secure}, absolute);
     }
 
     if (schema.isUser(this)) {
-        return config.urlFor('author', {author: this}, absolute);
+        return config.urlFor('author', {author: this, secure: this.secure}, absolute);
     }
 
     if (schema.isNav(this)) {
-        return config.urlFor('nav', {nav: this}, absolute);
+        return config.urlFor('nav', {nav: this, secure: this.secure}, absolute);
     }
 
-    return config.urlFor(this, absolute);
+    return config.urlFor(this, {}, absolute);
 };
 
 module.exports = url;

core/test/functional/routes/frontend_spec.js

@@ -985,11 +985,11 @@ describe('Frontend Routing', function () {
                 .end(doEnd(done));
         });
 
-        it('should set links to urlSSL over HTTPS', function (done) {
+        it('should set links to urlSSL over HTTPS besides canonical', function (done) {
             request.get('/')
                 .set('X-Forwarded-Proto', 'https')
                 .expect(200)
-                .expect(/<link rel="canonical" href="https:\/\/localhost\/" \/\>/)
+                .expect(/<link rel="canonical" href="http:\/\/127.0.0.1:2370\/" \/\>/)
                 .expect(/<a href="https:\/\/localhost">Ghost<\/a\>/)
                 .end(doEnd(done));
         });

core/test/unit/server_helpers/url_spec.js

@@ -64,6 +64,28 @@ describe('{{url}} helper', function () {
         rendered.should.equal('http://testurl.com/slug/');
     });
 
+    it('should output an absolute URL with https if the option is present and secure', function () {
+        rendered = helpers.url.call(
+            {html: 'content', markdown: 'ff', title: 'title', slug: 'slug',
+            url: '/slug/', created_at: new Date(0), secure: true},
+            {hash: {absolute: 'true'}}
+        );
+
+        should.exist(rendered);
+        rendered.should.equal('https://testurl.com/slug/');
+    });
+
+    it('should output an absolute URL with https if secure', function () {
+        rendered = helpers.url.call(
+            {html: 'content', markdown: 'ff', title: 'title', slug: 'slug',
+            url: '/slug/', created_at: new Date(0), secure: true},
+            {hash: {absolute: 'true'}}
+        );
+
+        should.exist(rendered);
+        rendered.should.equal('https://testurl.com/slug/');
+    });
+
     it('should return the slug with a prefixed /tag/ if the context is a tag', function () {
         rendered = helpers.url.call({
             name: 'the tag',
@@ -109,6 +131,14 @@ describe('{{url}} helper', function () {
         rendered.should.equal('http://testurl.com/bar');
     });
 
+    it('should return an absolute url with https if context is secure', function () {
+        rendered = helpers.url.call(
+            {url: '/bar', label: 'Bar', slug: 'bar', current: true, secure: true},
+            {hash: {absolute: 'true'}});
+        should.exist(rendered);
+        rendered.should.equal('https://testurl.com/bar');
+    });
+
     it('external urls should be retained in a nav context', function () {
         rendered = helpers.url.call(
             {url: 'http://casper.website/baz', label: 'Baz', slug: 'baz', current: true},
@@ -125,6 +155,24 @@ describe('{{url}} helper', function () {
         rendered.should.equal('http://testurl.com/qux');
     });
 
+    it('should handle hosted urls in a nav context with secure', function () {
+        rendered = helpers.url.call(
+            {url: 'http://testurl.com/qux', label: 'Qux', slug: 'qux', current: true,
+            secure: true},
+            {hash: {absolute: 'true'}});
+        should.exist(rendered);
+        rendered.should.equal('https://testurl.com/qux');
+    });
+
+    it('should handle hosted https urls in a nav context with secure', function () {
+        rendered = helpers.url.call(
+            {url: 'https://testurl.com/qux', label: 'Qux', slug: 'qux', current: true,
+            secure: true},
+            {hash: {absolute: 'true'}});
+        should.exist(rendered);
+        rendered.should.equal('https://testurl.com/qux');
+    });
+
     it('should handle hosted urls with the wrong protocol in a nav context', function () {
         rendered = helpers.url.call(
             {url: 'https://testurl.com/quux', label: 'Quux', slug: 'quux', current: true},