Fixed ETag header for admin templates not changing between versions (#13680)

refs https://github.com/TryGhost/Team/issues/1175

We found the ETag header sent when serving the Admin template for /ghost/ was not changing between versions which after an upgrade could result in out of date cached content being served containing links to JS/CSS files that no longer existed.

The culprit is weak etags served by Node's `send` package, coupled with Admin template filesize not changing between versions and `npm pack` setting a fixed modification date for every file. See https://github.com/pillarjs/send/issues/176 for more details.

- updated the Admin app's controller to read the template and generate an md5 hash of the contents so we can serve a strong ETag header value when serving the `/ghost/` html

core/server/web/admin/controller.js

@@ -1,5 +1,7 @@
 const debug = require('@tryghost/debug')('web:admin:controller');
 const path = require('path');
+const fs = require('fs');
+const crypto = require('crypto');
 const config = require('../../../shared/config');
 const updateCheck = require('../../update-check');
 
@@ -21,6 +23,15 @@ module.exports = function adminController(req, res) {
     const templatePath = path.resolve(config.get('paths').adminViews, defaultTemplate);
     const headers = {};
 
+    // Generate our own ETag header
+    //   `sendFile` by default uses filesize+lastmod date to generate an etag.
+    //   That doesn't work for admin templates because the filesize doesn't change between versions
+    //   and `npm pack` sets a fixed lastmod date for every file meaning the default etag never changes
+    const fileBuffer = fs.readFileSync(templatePath);
+    const hashSum = crypto.createHash('md5');
+    hashSum.update(fileBuffer);
+    headers.ETag = hashSum.digest('hex');
+
     if (config.get('adminFrameProtection')) {
         headers['X-Frame-Options'] = 'sameorigin';
     }

test/e2e-server/admin.test.js

@@ -4,6 +4,8 @@
 // But then again testing real code, rather than mock code, might be more useful...
 
 const should = require('should');
+const path = require('path');
+const fs = require('fs');
 
 const supertest = require('supertest');
 const testUtils = require('../utils');
@@ -28,14 +30,14 @@ describe('Admin Routing', function () {
 
     describe('Assets', function () {
         it('should return 404 for unknown assets', async function () {
-            request.get('/ghost/assets/not-found.js')
+            await request.get('/ghost/assets/not-found.js')
                 .expect('Cache-Control', testUtils.cacheRules.private)
                 .expect(404)
                 .expect(assertCorrectHeaders);
         });
 
         it('should retrieve built assets', async function () {
-            request.get('/ghost/assets/vendor.js')
+            await request.get('/ghost/assets/vendor.js')
                 .expect('Cache-Control', testUtils.cacheRules.year)
                 .expect(200)
                 .expect(assertCorrectHeaders);
@@ -44,7 +46,7 @@ describe('Admin Routing', function () {
 
     describe('Admin Redirects', function () {
         it('should redirect /GHOST/ to /ghost/', async function () {
-            request.get('/GHOST/')
+            await request.get('/GHOST/')
                 .expect('Location', '/ghost/')
                 .expect(301)
                 .expect(assertCorrectHeaders);
@@ -67,17 +69,60 @@ describe('Admin Routing', function () {
         });
 
         it('should redirect admin access over non-HTTPS', async function () {
-            request.get('/ghost/')
+            await request.get('/ghost/')
                 .expect('Location', /^https:\/\/localhost:2390\/ghost\//)
                 .expect(301)
                 .expect(assertCorrectHeaders);
         });
 
         it('should allow admin access over HTTPS', async function () {
-            request.get('/ghost/')
+            await request.get('/ghost/')
                 .set('X-Forwarded-Proto', 'https')
                 .expect(200)
                 .expect(assertCorrectHeaders);
         });
     });
+
+    describe('built template', function () {
+        beforeEach(function () {
+            const configPaths = configUtils.config.get('paths');
+            configPaths.adminViews = path.resolve('test/utils/fixtures/admin-views');
+            configUtils.set('paths', configPaths);
+        });
+
+        afterEach(function () {
+            configUtils.restore();
+        });
+
+        it('serves prod file in production', async function () {
+            configUtils.set('env', 'production');
+
+            const prodTemplate = fs.readFileSync(path.resolve('test/utils/fixtures/admin-views/default-prod.html')).toString();
+
+            const res = await request.get('/ghost/')
+                .set('X-Forwarded-Proto', 'https')
+                .expect(200);
+
+            res.text.should.equal(prodTemplate);
+        });
+
+        it('serves dev file when not in production', async function () {
+            const devTemplate = fs.readFileSync(path.resolve('test/utils/fixtures/admin-views/default.html')).toString();
+
+            const res = await request.get('/ghost/')
+                .set('X-Forwarded-Proto', 'https')
+                .expect(200);
+
+            res.text.should.equal(devTemplate);
+        });
+
+        it('generates it\'s own ETag header from file contents', async function () {
+            const res = await request.get('/ghost/')
+                .set('X-Forwarded-Proto', 'https')
+                .expect(200);
+
+            should.exist(res.headers.etag);
+            res.headers.etag.should.equal('b448e5380dbfc46bc7c6da6045bf3043');
+        });
+    });
 });

test/utils/fixtures/admin-views/default-prod.html

@@ -0,0 +1,4 @@
+<html>
+    <head><title>Prod template</title></head>
+    <body>Prod template</body>
+</html>

test/utils/fixtures/admin-views/default.html

@@ -0,0 +1,4 @@
+<html>
+    <head><title>Dev template</title></head>
+    <body>Dev template</body>
+</html>