Merge pull request #8151 from epixa/headercheck

Separate xsrf handling and version checking

src/server/http/__tests__/version_check.js

@@ -0,0 +1,76 @@
+import expect from 'expect.js';
+import { fromNode } from 'bluebird';
+import { resolve } from 'path';
+import * as kbnTestServer from '../../../../test/utils/kbn_server';
+
+const src = resolve.bind(null, __dirname, '../../../../src');
+
+const versionHeader = 'kbn-version';
+const version = require(src('../package.json')).version;
+
+describe('version_check request filter', function () {
+  function makeRequest(kbnServer, opts) {
+    return fromNode(cb => {
+      kbnTestServer.makeRequest(kbnServer, opts, (resp) => {
+        cb(null, resp);
+      });
+    });
+  }
+
+  async function makeServer() {
+    const kbnServer = kbnTestServer.createServer();
+
+    await kbnServer.ready();
+
+    kbnServer.server.route({
+      path: '/version_check/test/route',
+      method: 'GET',
+      handler: function (req, reply) {
+        reply(null, 'ok');
+      }
+    });
+
+    return kbnServer;
+  };
+
+  let kbnServer;
+  beforeEach(async () => kbnServer = await makeServer());
+  afterEach(async () => await kbnServer.close());
+
+  it('accepts requests with the correct version passed in the version header', async function () {
+    const resp = await makeRequest(kbnServer, {
+      url: '/version_check/test/route',
+      method: 'GET',
+      headers: {
+        [versionHeader]: version,
+      },
+    });
+
+    expect(resp.statusCode).to.be(200);
+    expect(resp.payload).to.be('ok');
+  });
+
+  it('rejects requests with an incorrect version passed in the version header', async function () {
+    const resp = await makeRequest(kbnServer, {
+      url: '/version_check/test/route',
+      method: 'GET',
+      headers: {
+        [versionHeader]: `invalid:${version}`,
+      },
+    });
+
+    expect(resp.statusCode).to.be(400);
+    expect(resp.headers).to.have.property(versionHeader, version);
+    expect(resp.payload).to.match(/"Browser client is out of date/);
+  });
+
+  it('accepts requests that do not include a version header', async function () {
+    const resp = await makeRequest(kbnServer, {
+      url: '/version_check/test/route',
+      method: 'GET'
+    });
+
+    expect(resp.statusCode).to.be(200);
+    expect(resp.payload).to.be('ok');
+  });
+});

src/server/http/__tests__/xsrf.js

@@ -7,8 +7,9 @@ const nonDestructiveMethods = ['GET', 'HEAD'];
 const destructiveMethods = ['POST', 'PUT', 'DELETE'];
 const src = resolve.bind(null, __dirname, '../../../../src');
 
-const xsrfHeader = 'kbn-version';
-const version = require(src('../package.json')).version;
+const xsrfHeader = 'kbn-xsrf';
+const versionHeader = 'kbn-version';
+const actualVersion = require(src('../package.json')).version;
 
 describe('xsrf request filter', function () {
   function inject(kbnServer, opts) {
@@ -57,31 +58,30 @@ describe('xsrf request filter', function () {
         else expect(resp.payload).to.be('ok');
       });
 
-      it('failes on invalid tokens', async function () {
+      it('accepts requests with the xsrf header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
           method: method,
           headers: {
-            [xsrfHeader]: `invalid:${version}`,
+            [xsrfHeader]: 'anything',
           },
         });
 
-        expect(resp.statusCode).to.be(400);
-        expect(resp.headers).to.have.property(xsrfHeader, version);
+        expect(resp.statusCode).to.be(200);
         if (method === 'HEAD') expect(resp.payload).to.be.empty();
-        else expect(resp.payload).to.match(/"Browser client is out of date/);
+        else expect(resp.payload).to.be('ok');
       });
     });
   }
 
   for (const method of destructiveMethods) {
     context(`destructiveMethod: ${method}`, function () { // eslint-disable-line no-loop-func
-      it('accepts requests with the correct token', async function () {
+      it('accepts requests with the xsrf header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
           method: method,
           headers: {
-            [xsrfHeader]: version,
+            [xsrfHeader]: 'anything',
           },
         });
 
@@ -89,27 +89,29 @@ describe('xsrf request filter', function () {
         expect(resp.payload).to.be('ok');
       });
 
-      it('rejects requests without a token', async function () {
+      // this is still valid for existing csrf protection support
+      // it does not actually do any validation on the version value itself
+      it('accepts requests with the version header', async function () {
+        const resp = await inject(kbnServer, {
+          url: '/xsrf/test/route',
+          method: method,
+          headers: {
+            [versionHeader]: actualVersion,
+          },
+        });
+
+        expect(resp.statusCode).to.be(200);
+        expect(resp.payload).to.be('ok');
+      });
+
+      it('rejects requests without either an xsrf or version header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
           method: method
         });
 
         expect(resp.statusCode).to.be(400);
-        expect(resp.payload).to.match(/"Missing kbn-version header/);
-      });
-
-      it('rejects requests with an invalid token', async function () {
-        const resp = await inject(kbnServer, {
-          url: '/xsrf/test/route',
-          method: method,
-          headers: {
-            [xsrfHeader]: `invalid:${version}`,
-          },
-        });
-
-        expect(resp.statusCode).to.be(400);
-        expect(resp.payload).to.match(/"Browser client is out of date/);
+        expect(resp.payload).to.match(/"Request must contain an kbn-xsrf header/);
       });
     });
   }

src/server/http/index.js

@@ -5,6 +5,8 @@ import fs from 'fs';
 import Boom from 'boom';
 import Hapi from 'hapi';
 import getDefaultRoute from './get_default_route';
+import versionCheckMixin from './version_check';
+
 module.exports = async function (kbnServer, server, config) {
 
 
@@ -132,5 +134,7 @@ module.exports = async function (kbnServer, server, config) {
     }
   });
 
+  kbnServer.mixin(versionCheckMixin);
+
   return kbnServer.mixin(require('./xsrf'));
 };

src/server/http/version_check.js

@@ -0,0 +1,19 @@
+import { badRequest } from 'boom';
+
+export default function (kbnServer, server, config) {
+  const versionHeader = 'kbn-version';
+  const actualVersion = config.get('pkg.version');
+
+  server.ext('onPostAuth', function (req, reply) {
+    const versionRequested = req.headers[versionHeader];
+
+    if (versionRequested && versionRequested !== actualVersion) {
+      return reply(badRequest('Browser client is out of date, please refresh the page', {
+        expected: actualVersion,
+        got: versionRequested
+      }));
+    }
+
+    return reply.continue();
+  });
+}

src/server/http/xsrf.js

@@ -1,21 +1,21 @@
 import { badRequest } from 'boom';
 
 export default function (kbnServer, server, config) {
-  const version = config.get('pkg.version');
   const disabled = config.get('server.xsrf.disableProtection');
-  const header = 'kbn-version';
+  const versionHeader = 'kbn-version';
+  const xsrfHeader = 'kbn-xsrf';
 
   server.ext('onPostAuth', function (req, reply) {
-    const noHeaderGet = (req.method === 'get' || req.method === 'head') && !req.headers[header];
-    if (disabled || noHeaderGet) return reply.continue();
+    if (disabled) {
+      return reply.continue();
+    }
 
-    const submission = req.headers[header];
-    if (!submission) return reply(badRequest(`Missing ${header} header`));
-    if (submission !== version) {
-      return reply(badRequest('Browser client is out of date, please refresh the page', {
-        expected: version,
-        got: submission
-      }));
+    const isSafeMethod = req.method === 'get' || req.method === 'head';
+    const hasVersionHeader = versionHeader in req.headers;
+    const hasXsrfHeader = xsrfHeader in req.headers;
+
+    if (!isSafeMethod && !hasVersionHeader && !hasXsrfHeader) {
+      return reply(badRequest(`Request must contain an ${xsrfHeader} header`));
     }
 
     return reply.continue();