🐛Fixed asset redirects (#9455)

closes #9445

- redirects all asset requests if https is configured (theme, core, images)
- re-use and extend our url-redirect middleware
- add proper integration tests for our express site app (no db interaction, component testing required for such important use cases)
- i added some more general tests
- should avoid mixed content warnings in the browser

core/server/web/middleware/url-redirects.js

@@ -1,4 +1,5 @@
 var url = require('url'),
+    path = require('path'),
     debug = require('ghost-ignition').debug('url-redirects'),
     urlService = require('../../services/url'),
     urlRedirects,
@@ -6,21 +7,22 @@ var url = require('url'),
 
 _private.redirectUrl = function redirectUrl(options) {
     var redirectTo = options.redirectTo,
-        path = options.path,
+        pathname = options.path,
         query = options.query,
         parts = url.parse(redirectTo);
 
     // CASE: ensure we always add a trailing slash to reduce the number of redirects
     // e.g. you are redirected from example.com/ghost to admin.example.com/ghost and Ghost would detect a missing slash and redirect you to /ghost/
-    if (!path.match(/\/$/)) {
-        path += '/';
+    // Exceptions: asset requests
+    if (!pathname.match(/\/$/) && !path.extname(pathname)) {
+        pathname += '/';
     }
 
     return url.format({
         protocol: parts.protocol,
         hostname: parts.hostname,
         port: parts.port,
-        pathname: path,
+        pathname: pathname,
         query: query
     });
 };

core/server/web/site/app.js

@@ -44,9 +44,14 @@ module.exports = function setupSiteApp() {
     // you can extend Ghost with a custom redirects file
     // see https://github.com/TryGhost/Ghost/issues/7707
     customRedirects.use(siteApp);
+
     // More redirects
     siteApp.use(adminRedirects());
 
+    // force SSL if blog url is set to https. The redirects handling must happen before asset and page routing,
+    // otherwise we serve assets/pages with http. This can cause mixed content warnings in the admin client.
+    siteApp.use(urlRedirects);
+
     // Static content/assets
     // @TODO make sure all of these have a local 404 error handler
     // Favicon
@@ -105,10 +110,6 @@ module.exports = function setupSiteApp() {
     // send 503 error page in case of maintenance
     siteApp.use(maintenance);
 
-    // Force SSL if required
-    // must happen AFTER asset loading and BEFORE routing
-    siteApp.use(urlRedirects);
-
     // Add in all trailing slashes & remove uppercase
     // must happen AFTER asset loading and BEFORE routing
     siteApp.use(prettyURLs);

core/test/integration/site_spec.js

@@ -0,0 +1,182 @@
+'use strict';
+
+const should = require('should'), // jshint ignore:line
+    sinon = require('sinon'),
+    testUtils = require('../utils/index'),
+    configUtils = require('../utils/configUtils'),
+    siteApp = require('../../server/web/site/app'),
+    models = require('../../server/models'),
+    sandbox = sinon.sandbox.create();
+
+describe('Integration - Web - Site', function () {
+    let app;
+
+    beforeEach(function () {
+        app = siteApp();
+
+        return testUtils.configureGhost(sandbox);
+    });
+
+    afterEach(function () {
+        sandbox.restore();
+        configUtils.restore();
+    });
+
+    describe('component: prettify', function () {
+        it('url without slash', function () {
+            const req = {
+                secure: false,
+                method: 'GET',
+                url: '/prettify-me',
+                host: 'example.com'
+            };
+
+            return testUtils.mocks.express.invoke(app, req)
+                .then(function (response) {
+                    response.statusCode.should.eql(301);
+                    response.headers.location.should.eql('/prettify-me/');
+                });
+        });
+    });
+
+    describe('component: url redirects', function () {
+        describe('page', function () {
+            it('success', function () {
+                configUtils.set('url', 'https://example.com');
+
+                sandbox.stub(models.Post, 'findOne')
+                    .resolves(models.Post.forge(testUtils.DataGenerator.forKnex.createPost({slug: 'cars'})));
+
+                const req = {
+                    secure: true,
+                    method: 'GET',
+                    url: '/cars/',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(200);
+                        response.template.should.eql('post');
+                    });
+            });
+
+            it('blog is https, request is http', function () {
+                configUtils.set('url', 'https://example.com');
+
+                const req = {
+                    secure: false,
+                    host: 'example.com',
+                    method: 'GET',
+                    url: '/cars'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(301);
+                        response.headers.location.should.eql('https://example.com/cars/');
+                    });
+            });
+
+            it('blog is https, request is http, trailing slash exists already', function () {
+                configUtils.set('url', 'https://example.com');
+
+                const req = {
+                    secure: false,
+                    method: 'GET',
+                    url: '/cars/',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(301);
+                        response.headers.location.should.eql('https://example.com/cars/');
+                    });
+            });
+        });
+
+        describe('assets', function () {
+            it('success', function () {
+                const req = {
+                    secure: false,
+                    method: 'GET',
+                    url: '/public/ghost-sdk.js',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(200);
+                    });
+            });
+
+            it('success', function () {
+                configUtils.set('url', 'https://example.com');
+
+                const req = {
+                    secure: true,
+                    method: 'GET',
+                    url: '/assets/css/screen.css',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(200);
+                    });
+            });
+
+            it('blog is https, request is http', function () {
+                configUtils.set('url', 'https://example.com');
+
+                const req = {
+                    secure: false,
+                    method: 'GET',
+                    url: '/public/ghost-sdk.js',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(301);
+                        response.headers.location.should.eql('https://example.com/public/ghost-sdk.js');
+                    });
+            });
+
+            it('blog is https, request is http', function () {
+                configUtils.set('url', 'https://example.com');
+
+                const req = {
+                    secure: false,
+                    method: 'GET',
+                    url: '/favicon.png',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(301);
+                        response.headers.location.should.eql('https://example.com/favicon.png');
+                    });
+            });
+
+            it('blog is https, request is http', function () {
+                configUtils.set('url', 'https://example.com');
+
+                const req = {
+                    secure: false,
+                    method: 'GET',
+                    url: '/assets/css/main.css',
+                    host: 'example.com'
+                };
+
+                return testUtils.mocks.express.invoke(app, req)
+                    .then(function (response) {
+                        response.statusCode.should.eql(301);
+                        response.headers.location.should.eql('https://example.com/assets/css/main.css');
+                    });
+            });
+        });
+    });
+});

core/test/utils/index.js

@@ -47,6 +47,7 @@ var Promise = require('bluebird'),
     login,
     togglePermalinks,
     startGhost,
+    configureGhost,
 
     initFixtures,
     initData,
@@ -918,8 +919,28 @@ startGhost = function startGhost(options) {
         });
 };
 
+/**
+ * Minimal configuration to start integration/unit tests.
+ */
+configureGhost = function configureGhost(sandbox) {
+    models.init();
+
+    const cacheStub = sandbox.stub(SettingsCache, 'get');
+
+    cacheStub.withArgs('active_theme').returns('casper');
+    cacheStub.withArgs('active_timezone').returns('Etc/UTC');
+    cacheStub.withArgs('permalinks').returns('/:slug/');
+
+    configUtils.set('paths:contentPath', path.join(__dirname, 'fixtures'));
+
+    configUtils.set('times:getImageSizeTimeoutInMS', 1);
+
+    return themes.init();
+};
+
 module.exports = {
     startGhost: startGhost,
+    configureGhost: configureGhost,
     teardown: teardown,
     setup: setup,
     doAuth: doAuth,

core/test/utils/mocks/express.js

@@ -0,0 +1,51 @@
+'use strict';
+
+const _ = require('lodash');
+const http = require('http');
+
+module.exports = {
+    invoke: function (app, reqParams) {
+        let req = new http.IncomingMessage();
+        let res = new http.ServerResponse({
+            method: reqParams.method
+        });
+
+        res.end = function () {
+            this.emit('finish');
+        };
+
+        req.connection = {
+            encrypted: reqParams.secure
+        };
+
+        req.method = 'GET';
+        req.url = reqParams.url;
+        req.headers = {
+            host: reqParams.host
+        };
+
+        res.connection = {
+            _httpMessage: res,
+            writable: true,
+            destroyed: false,
+            cork: function () {},
+            uncork: function () {},
+            write: function () {}
+        };
+
+        return new Promise(function (resolve) {
+            const onFinish = (() => {
+                resolve({
+                    statusCode: res.statusCode,
+                    headers: res._headers,
+                    template: res._template,
+                    req: req,
+                    res: res
+                });
+            });
+
+            res.once('finish', onFinish);
+            app(req, res);
+        });
+    }
+};

core/test/utils/mocks/index.js

@@ -1 +1,2 @@
 exports.utils = require('./utils');
+exports.express = require('./express');