[server/rewriteBasePath] Support rewriting basePath requests (#16724)

* [server/rewriteBasePath] add option to enable basePath rewriting

* [server/rewriteBasePath/docs] end sentences with periods

* [server/rewriteBasePath] simplify Joi schema a smidge

* [server/rewriteBasePath] rename test file to match source

* [server/rewriteBasePath] initialize server in before/after hooks

* [server/rewriteBasePath] rephrase deprecation warning

* [server/config/schema] verify that non-strings are not accepted for basePath

* [server/config/schema] toss a trailing comma in there

config/kibana.yml

@@ -6,11 +6,18 @@
 # To allow connections from remote users, set this parameter to a non-loopback address.
 #server.host: "localhost"
 
-# Enables you to specify a path to mount Kibana at if you are running behind a proxy. This only affects
-# the URLs generated by Kibana, your proxy is expected to remove the basePath value before forwarding requests
-# to Kibana. This setting cannot end in a slash.
+# Enables you to specify a path to mount Kibana at if you are running behind a proxy.
+# Use the `server.rewriteBasePath` setting to tell Kibana if it should remove the basePath
+# from requests it receives, and to prevent a deprecation warning at startup.
+# This setting cannot end in a slash.
 #server.basePath: ""
 
+# Specifies whether Kibana should rewrite requests that are prefixed with
+# `server.basePath` or require that they are rewritten by your reverse proxy.
+# This setting was effectively always `false` before Kibana 6.3 and will
+# default to `true` starting in Kibana 7.0.
+#server.rewriteBasePath: false
+
 # The maximum payload size in bytes for incoming server requests.
 #server.maxPayloadBytes: 1048576
 

docs/setup/settings.asciidoc

@@ -84,9 +84,8 @@ The following example shows a valid regionmap configuration.
 `regionmap.includeElasticMapsService:`:: turns on or off whether layers from the Elastic Maps Service should be included in the vector layer option list.
 By turning this off, only the layers that are configured here will be included. The default is true.
 
-`server.basePath:`:: Enables you to specify a path to mount Kibana at if you are running behind a proxy. This only affects
- the URLs generated by Kibana, your proxy is expected to remove the basePath value before forwarding requests
- to Kibana. This setting cannot end in a slash (`/`).
+`server.basePath:`:: Enables you to specify a path to mount Kibana at if you are running behind a proxy. Use the `server.rewriteBasePath` setting to tell Kibana if it should remove the basePath from requests it receives, and to prevent a deprecation warning at startup. This setting cannot end in a slash (`/`).
+`server.rewriteBasePath:`:: *Default: false* Specifies whether Kibana should rewrite requests that are prefixed with `server.basePath` or require that they are rewritten by your reverse proxy. This setting was effectively always `false` before Kibana 6.3 and will default to `true` starting in Kibana 7.0.
 `server.customResponseHeaders:`:: *Default: `{}`* Header names and values to send on all responses to the client from the Kibana server.
 `server.defaultRoute:`:: *Default: "/app/kibana"* This setting specifies the default route when opening Kibana. You can use this setting to modify the landing page when opening Kibana.
 `server.host:`:: *Default: "localhost"* This setting specifies the host of the back end server.

src/cli/cluster/base_path_proxy.js

@@ -1,7 +1,6 @@
 import { Server } from 'hapi';
 import { notFound } from 'boom';
 import { map, sample } from 'lodash';
-import { format as formatUrl } from 'url';
 import { map as promiseMap, fromNode } from 'bluebird';
 import { Agent as HttpsAgent } from 'https';
 import { readFileSync } from 'fs';
@@ -92,15 +91,9 @@ export default class BasePathProxy {
           passThrough: true,
           xforward: true,
           agent: this.proxyAgent,
-          mapUri(req, callback) {
-            callback(null, formatUrl({
-              protocol: server.info.protocol,
-              hostname: server.info.host,
-              port: targetPort,
-              pathname: req.params.kbnPath,
-              query: req.query,
-            }));
-          }
+          protocol: server.info.protocol,
+          host: server.info.host,
+          port: targetPort,
         }
       }
     });

src/cli/cluster/cluster_manager.js

@@ -22,12 +22,14 @@ export default class ClusterManager {
       this.basePathProxy = new BasePathProxy(this, settings);
 
       optimizerArgv.push(
-        `--server.basePath=${this.basePathProxy.basePath}`
+        `--server.basePath=${this.basePathProxy.basePath}`,
+        '--server.rewriteBasePath=true',
       );
 
       serverArgv.push(
         `--server.port=${this.basePathProxy.targetPort}`,
-        `--server.basePath=${this.basePathProxy.basePath}`
+        `--server.basePath=${this.basePathProxy.basePath}`,
+        '--server.rewriteBasePath=true',
       );
     }
 

src/server/config/__tests__/schema.js

@@ -19,13 +19,15 @@ describe('Config schema', function () {
 
     describe('basePath', function () {
       it('accepts empty strings', function () {
-        const { error } = validate({ server: { basePath: '' } });
+        const { error, value } = validate({ server: { basePath: '' } });
         expect(error == null).to.be.ok();
+        expect(value.server.basePath).to.be('');
       });
 
       it('accepts strings with leading slashes', function () {
-        const { error } = validate({ server: { basePath: '/path' } });
+        const { error, value } = validate({ server: { basePath: '/path' } });
         expect(error == null).to.be.ok();
+        expect(value.server.basePath).to.be('/path');
       });
 
       it('rejects strings with trailing slashes', function () {
@@ -40,6 +42,45 @@ describe('Config schema', function () {
         expect(error.details[0]).to.have.property('path', 'server.basePath');
       });
 
+      it('rejects things that are not strings', function () {
+        for (const value of [1, true, {}, [], /foo/]) {
+          const { error } = validate({ server: { basePath: value } });
+          expect(error).to.have.property('details');
+          expect(error.details[0]).to.have.property('path', 'server.basePath');
+        }
+      });
+    });
+
+    describe('rewriteBasePath', function () {
+      it('defaults to false', () => {
+        const { error, value } = validate({});
+        expect(error).to.be(null);
+        expect(value.server.rewriteBasePath).to.be(false);
+      });
+
+      it('accepts false', function () {
+        const { error, value } = validate({ server: { rewriteBasePath: false } });
+        expect(error).to.be(null);
+        expect(value.server.rewriteBasePath).to.be(false);
+      });
+
+      it('accepts true if basePath set', function () {
+        const { error, value } = validate({ server: { basePath: '/foo', rewriteBasePath: true } });
+        expect(error).to.be(null);
+        expect(value.server.rewriteBasePath).to.be(true);
+      });
+
+      it('rejects true if basePath not set', function () {
+        const { error } = validate({ server: { rewriteBasePath: true } });
+        expect(error).to.have.property('details');
+        expect(error.details[0]).to.have.property('path', 'server.rewriteBasePath');
+      });
+
+      it('rejects strings', function () {
+        const { error } = validate({ server: { rewriteBasePath: 'foo' } });
+        expect(error).to.have.property('details');
+        expect(error.details[0]).to.have.property('path', 'server.rewriteBasePath');
+      });
     });
 
     describe('ssl', function () {

src/server/config/schema.js

@@ -53,6 +53,11 @@ export default () => Joi.object({
     autoListen: Joi.boolean().default(true),
     defaultRoute: Joi.string().default('/app/kibana').regex(/^\//, `start with a slash`),
     basePath: Joi.string().default('').allow('').regex(/(^$|^\/.*[^\/]$)/, `start with a slash, don't end with one`),
+    rewriteBasePath: Joi.boolean().when('basePath', {
+      is: '',
+      then: Joi.default(false).valid(false),
+      otherwise: Joi.default(false),
+    }),
     customResponseHeaders: Joi.object().unknown(true).default({}),
     ssl: Joi.object({
       enabled: Joi.boolean().default(false),

src/server/config/transform_deprecations.js

@@ -25,6 +25,17 @@ const savedObjectsIndexCheckTimeout = (settings, log) => {
   }
 };
 
+const rewriteBasePath = (settings, log) => {
+  if (_.has(settings, 'server.basePath') && !_.has(settings, 'server.rewriteBasePath')) {
+    log(
+      'You should set server.basePath along with server.rewriteBasePath. Starting in 7.0, Kibana ' +
+      'will expect that all requests start with server.basePath rather than expecting you to rewrite ' +
+      'the requests in your reverse proxy. Set server.rewriteBasePath to false to preserve the ' +
+      'current behavior and silence this warning.'
+    );
+  }
+};
+
 const deprecations = [
   //server
   rename('server.ssl.cert', 'server.ssl.certificate'),
@@ -37,6 +48,7 @@ const deprecations = [
   rename('optimize.lazyProxyTimeout', 'optimize.watchProxyTimeout'),
   serverSslEnabled,
   savedObjectsIndexCheckTimeout,
+  rewriteBasePath,
 ];
 
 export const transformDeprecations = createTransform(deprecations);

src/server/http/__tests__/setup_base_path_rewrite.js

@@ -0,0 +1,185 @@
+import { Server } from 'hapi';
+import expect from 'expect.js';
+import sinon from 'sinon';
+
+import { setupBasePathRewrite } from '../setup_base_path_rewrite';
+
+describe('server / setup_base_path_rewrite', () => {
+  function createServer({ basePath, rewriteBasePath }) {
+    const config = {
+      get: sinon.stub()
+    };
+
+    config.get.withArgs('server.basePath')
+      .returns(basePath);
+    config.get.withArgs('server.rewriteBasePath')
+      .returns(rewriteBasePath);
+
+    const server = new Server();
+    server.connection({ port: 0 });
+    setupBasePathRewrite({}, server, config);
+
+    server.route({
+      method: 'GET',
+      path: '/',
+      handler(req, reply) {
+        reply('resp:/');
+      }
+    });
+
+    server.route({
+      method: 'GET',
+      path: '/foo',
+      handler(req, reply) {
+        reply('resp:/foo');
+      }
+    });
+
+    return server;
+  }
+
+  describe('no base path', () => {
+    let server;
+    before(() => server = createServer({ basePath: '', rewriteBasePath: false }));
+    after(() => server = undefined);
+
+    it('/bar => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/bar/ => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar/'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/bar/foo => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar/foo'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/ => /', async () => {
+      const resp = await server.inject({
+        url: '/'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/');
+    });
+
+    it('/foo => /foo', async () => {
+      const resp = await server.inject({
+        url: '/foo'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/foo');
+    });
+  });
+
+  describe('base path /bar, rewrite = false', () => {
+    let server;
+    before(() => server = createServer({ basePath: '/bar', rewriteBasePath: false }));
+    after(() => server = undefined);
+
+    it('/bar => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/bar/ => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar/'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/bar/foo => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar/foo'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/ => /', async () => {
+      const resp = await server.inject({
+        url: '/'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/');
+    });
+
+    it('/foo => /foo', async () => {
+      const resp = await server.inject({
+        url: '/foo'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/foo');
+    });
+  });
+
+  describe('base path /bar, rewrite = true', () => {
+    let server;
+    before(() => server = createServer({ basePath: '/bar', rewriteBasePath: true }));
+    after(() => server = undefined);
+
+    it('/bar => /', async () => {
+      const resp = await server.inject({
+        url: '/bar'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/');
+    });
+
+    it('/bar/ => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar/'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/');
+    });
+
+    it('/bar/foo => 404', async () => {
+      const resp = await server.inject({
+        url: '/bar/foo'
+      });
+
+      expect(resp.statusCode).to.be(200);
+      expect(resp.payload).to.be('resp:/foo');
+    });
+
+    it('/ => 404', async () => {
+      const resp = await server.inject({
+        url: '/'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+
+    it('/foo => 404', async () => {
+      const resp = await server.inject({
+        url: '/foo'
+      });
+
+      expect(resp.statusCode).to.be(404);
+    });
+  });
+});

src/server/http/index.js

@@ -10,6 +10,7 @@ import shortUrlLookupProvider from './short_url_lookup';
 import setupConnectionMixin from './setup_connection';
 import setupRedirectMixin from './setup_redirect_server';
 import registerHapiPluginsMixin from './register_hapi_plugins';
+import { setupBasePathRewrite } from './setup_base_path_rewrite';
 import xsrfMixin from './xsrf';
 
 export default async function (kbnServer, server, config) {
@@ -17,6 +18,7 @@ export default async function (kbnServer, server, config) {
 
   const shortUrlLookup = shortUrlLookupProvider(server);
   await kbnServer.mixin(setupConnectionMixin);
+  await kbnServer.mixin(setupBasePathRewrite);
   await kbnServer.mixin(setupRedirectMixin);
   await kbnServer.mixin(registerHapiPluginsMixin);
 

src/server/http/setup_base_path_rewrite.js

@@ -0,0 +1,30 @@
+import Boom from 'boom';
+
+import { modifyUrl } from '../../utils';
+
+export function setupBasePathRewrite(kbnServer, server, config) {
+  const basePath = config.get('server.basePath');
+  const rewriteBasePath = config.get('server.rewriteBasePath');
+
+  if (!basePath || !rewriteBasePath) {
+    return;
+  }
+
+  server.ext('onRequest', (request, reply) => {
+    const newUrl = modifyUrl(request.url.href, parsed => {
+      if (parsed.pathname.startsWith(basePath)) {
+        parsed.pathname = parsed.pathname.replace(basePath, '') || '/';
+      } else {
+        return {};
+      }
+    });
+
+    if (!newUrl) {
+      reply(Boom.notFound());
+      return;
+    }
+
+    request.setUrl(newUrl);
+    reply.continue();
+  });
+}

src/server/kbn_server.js

@@ -106,7 +106,8 @@ export default class KbnServer {
    */
   async listen() {
     const {
-      server
+      server,
+      config,
     } = this;
 
     await this.ready();
@@ -117,7 +118,11 @@ export default class KbnServer {
       process.send(['WORKER_LISTENING']);
     }
 
-    server.log(['listening', 'info'], `Server running at ${server.info.uri}`);
+    server.log(['listening', 'info'], `Server running at ${server.info.uri}${
+      config.get('server.rewriteBasePath')
+        ? config.get('server.basePath')
+        : ''
+    }`);
     return server;
   }