Merge pull request #2953 from overleaf/jpa-nocache

[misc] Server: invoke the nocache middleware explicitly and add test

GitOrigin-RevId: 3238b07ebf5963ae95ef3f353e4745d283795fba

services/web/app/src/infrastructure/Server.js

@@ -195,18 +195,24 @@ webRouter.use(function(req, res, next) {
 })
 
 // add security headers using Helmet
+const noCacheMiddleware = require('nocache')()
 webRouter.use(function(req, res, next) {
   const isLoggedIn = AuthenticationController.isUserLoggedIn(req)
   const isProjectPage = !!req.path.match('^/project/[a-f0-9]{24}$')
-
+  if (isLoggedIn || isProjectPage) {
+    noCacheMiddleware(req, res, next)
+  } else {
+    next()
+  }
+})
+webRouter.use(
   helmet({
     // note that more headers are added by default
     dnsPrefetchControl: false,
     referrerPolicy: { policy: 'origin-when-cross-origin' },
-    noCache: isLoggedIn || isProjectPage,
     hsts: false
-  })(req, res, next)
+  })
-})
+)
 
 logger.info('creating HTTP server'.yellow)
 const server = require('http').createServer(app)

services/web/package.json

@@ -91,6 +91,7 @@
     "mongojs": "2.4.0",
     "mongoose": "^4.13.19",
     "multer": "git+https://github.com/overleaf/multer.git",
+    "nocache": "^2.1.0",
     "node-html-encoder": "0.0.2",
     "nodemailer": "2.1.0",
     "nodemailer-mandrill-transport": "^1.2.0",

services/web/test/acceptance/config/settings.test.coffee

@@ -9,6 +9,7 @@ httpAuthUsers = {}
 httpAuthUsers[httpAuthUser] = httpAuthPass
 
 module.exports =
+	cacheStaticAssets: true
 	enableSubscriptions: true
 
 	httpAuthUsers: httpAuthUsers

services/web/test/acceptance/src/SecurityHeadersTests.js

@@ -42,6 +42,10 @@ const assert_has_no_cache_headers = function(response) {
   assert.isUndefined(headers['pragma'])
   return assert.isUndefined(headers['expires'])
 }
+const assert_has_asset_caching_headers = function(response) {
+  const { headers } = response
+  assert.equal(headers['cache-control'], 'public, max-age=31536000')
+}
 
 describe('SecurityHeaders', function() {
   beforeEach(function() {
@@ -69,6 +73,13 @@ describe('SecurityHeaders', function() {
     })
   })
 
+  it('should have caching headers on static assets', function(done) {
+    request.get('/favicon.ico', (err, res) => {
+      assert_has_asset_caching_headers(res)
+      done(err)
+    })
+  })
+
   it('should have cache headers when user is logged in', function(done) {
     return async.series(
       [
@@ -110,4 +121,19 @@ describe('SecurityHeaders', function() {
       }
     )
   })
+
+  it('should have caching headers on static assets when user is logged in', function(done) {
+    async.series(
+      [
+        cb => this.user.login(cb),
+        cb => this.user.request.get('/favicon.ico', cb),
+        cb => this.user.logout(cb)
+      ],
+      (err, results) => {
+        const res = results[1][0]
+        assert_has_asset_caching_headers(res)
+        done()
+      }
+    )
+  })
 })