From ce6745b6a153ab5f183b9971d341c847c245bc1b Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Sun, 22 Sep 2019 18:20:05 +0100
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Improved=20x-request-id=20handling?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Currently, we create a request ID for internal use if one isn't set & this is used in logs
- If a custom request ID is set via X-Request-ID header, this gets logged, however, we don't return this with the response
- Means that a custom ID gets lost on the way back out, and makes tracing requests through a system trickier
- This change ensures that if X-Request-ID is set on the request, it is also set on the response so that requests can be properly traced
- It's easy to set this in e.g. nginx so that the feature becomes available - Ghost doens't need to do this
- Note: also split request id handling out into new middleware
---
 core/server/web/middleware/log-request.js | 10 +++-------
 core/server/web/middleware/request-id.js  | 18 ++++++++++++++++++
 core/server/web/parent-app.js             |  2 ++
 3 files changed, 23 insertions(+), 7 deletions(-)
 create mode 100644 core/server/web/middleware/request-id.js

diff --git a/core/server/web/middleware/log-request.js b/core/server/web/middleware/log-request.js
index b7682b539e..6ec6aeb062 100644
--- a/core/server/web/middleware/log-request.js
+++ b/core/server/web/middleware/log-request.js
@@ -1,17 +1,13 @@
-var uuid = require('uuid'),
-    common = require('../../lib/common');
+var common = require('../../lib/common');
 
 /**
- * @TODO:
- * - move middleware to ignition?
+ * @TODO: move this middleware to ignition?
  */
 module.exports = function logRequest(req, res, next) {
-    var startTime = Date.now(),
-        requestId = req.get('X-Request-ID') || uuid.v1();
+    var startTime = Date.now();
 
     function logResponse() {
         res.responseTime = (Date.now() - startTime) + 'ms';
-        req.requestId = requestId;
         req.userId = req.user ? (req.user.id ? req.user.id : req.user) : null;
 
         if (req.err && req.err.statusCode !== 404) {
diff --git a/core/server/web/middleware/request-id.js b/core/server/web/middleware/request-id.js
new file mode 100644
index 0000000000..be465fc3a2
--- /dev/null
+++ b/core/server/web/middleware/request-id.js
@@ -0,0 +1,18 @@
+const uuid = require('uuid');
+
+/**
+ * @TODO: move this middleware to ignition?
+ */
+module.exports = (req, res, next) => {
+    const requestId = req.get('X-Request-ID') || uuid.v4();
+
+    // Set a value for internal use
+    req.requestId = requestId;
+
+    // If the header was set on the request, return it on the response
+    if (req.get('X-Request-ID')) {
+        res.set('X-Request-ID', requestId);
+    }
+
+    next();
+};
diff --git a/core/server/web/parent-app.js b/core/server/web/parent-app.js
index 9aef6ece4c..bf84621c06 100644
--- a/core/server/web/parent-app.js
+++ b/core/server/web/parent-app.js
@@ -10,6 +10,7 @@ var debug = require('ghost-ignition').debug('app'),
 
     // local middleware
     ghostLocals = require('./middleware/ghost-locals'),
+    requestId = require('./middleware/request-id'),
     logRequest = require('./middleware/log-request');
 
 module.exports = function setupParentApp() {
@@ -22,6 +23,7 @@ module.exports = function setupParentApp() {
     // (X-Forwarded-Proto header will be checked, if present)
     parentApp.enable('trust proxy');
 
+    parentApp.use(requestId);
     parentApp.use(logRequest);
 
     // enabled gzip compression by default