From 3c07d0819faa73a5d5caad05e7c6c3af14e88c56 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 13 Feb 2018 17:59:54 -0700 Subject: [PATCH 1/8] [server/rewriteBasePath] add option to enable basePath rewriting --- config/kibana.yml | 13 +- docs/setup/settings.asciidoc | 5 +- src/cli/cluster/base_path_proxy.js | 13 +- src/cli/cluster/cluster_manager.js | 6 +- src/server/config/__tests__/schema.js | 37 +++- src/server/config/schema.js | 5 + src/server/config/transform_deprecations.js | 13 ++ .../http/__tests__/base_path_rewrite.js | 188 ++++++++++++++++++ src/server/http/index.js | 2 + src/server/http/setup_base_path_rewrite.js | 30 +++ src/server/kbn_server.js | 9 +- 11 files changed, 299 insertions(+), 22 deletions(-) create mode 100644 src/server/http/__tests__/base_path_rewrite.js create mode 100644 src/server/http/setup_base_path_rewrite.js diff --git a/config/kibana.yml b/config/kibana.yml index db9e0d60f9fc2e..33c1bcba2f93fc 100644 --- a/config/kibana.yml +++ b/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 diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 594cc31b2b14ed..44b5316335ce75 100644 --- a/docs/setup/settings.asciidoc +++ b/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. diff --git a/src/cli/cluster/base_path_proxy.js b/src/cli/cluster/base_path_proxy.js index 32e571901cf73b..63a40f47919f15 100644 --- a/src/cli/cluster/base_path_proxy.js +++ b/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, } } }); diff --git a/src/cli/cluster/cluster_manager.js b/src/cli/cluster/cluster_manager.js index 845cf5be6c6fa2..5862f212e14bf0 100644 --- a/src/cli/cluster/cluster_manager.js +++ b/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', ); } diff --git a/src/server/config/__tests__/schema.js b/src/server/config/__tests__/schema.js index 612fa070a4a5c4..a73e425f114bce 100644 --- a/src/server/config/__tests__/schema.js +++ b/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 () { @@ -39,7 +41,38 @@ describe('Config schema', function () { 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 () { diff --git a/src/server/config/schema.js b/src/server/config/schema.js index 07e859ada5c535..49c968249cfc30 100644 --- a/src/server/config/schema.js +++ b/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: Joi.valid(''), + then: Joi.default(false).valid(false), + otherwise: Joi.default(false) + }), customResponseHeaders: Joi.object().unknown(true).default({}), ssl: Joi.object({ enabled: Joi.boolean().default(false), diff --git a/src/server/config/transform_deprecations.js b/src/server/config/transform_deprecations.js index fb74522597f933..959a4c7de59866 100644 --- a/src/server/config/transform_deprecations.js +++ b/src/server/config/transform_deprecations.js @@ -25,6 +25,18 @@ const savedObjectsIndexCheckTimeout = (settings, log) => { } }; +const rewriteBasePath = (settings, log) => { + if (_.has(settings, 'server.basePath') && !_.has(settings, 'server.rewriteBasePath')) { + log( + 'Setting server.basePath without server.rewriteBasePath is deprecated until Kibana 7.0, ' + + 'when the default behavior of server.basePath will change and Kibana will start expecting ' + + 'that all requests start with the server.basePath rather 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 +49,7 @@ const deprecations = [ rename('optimize.lazyProxyTimeout', 'optimize.watchProxyTimeout'), serverSslEnabled, savedObjectsIndexCheckTimeout, + rewriteBasePath, ]; export const transformDeprecations = createTransform(deprecations); diff --git a/src/server/http/__tests__/base_path_rewrite.js b/src/server/http/__tests__/base_path_rewrite.js new file mode 100644 index 00000000000000..e329e495a80151 --- /dev/null +++ b/src/server/http/__tests__/base_path_rewrite.js @@ -0,0 +1,188 @@ +import { Server } from 'hapi'; +import expect from 'expect.js'; +import sinon from 'sinon'; + +import { setupBasePathRewrite } from '../setup_base_path_rewrite'; + +describe('server / base path rewriting', () => { + 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', () => { + it('/bar => 404', async () => { + const server = createServer({ basePath: '', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/bar' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/bar/ => 404', async () => { + const server = createServer({ basePath: '', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/bar/' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/bar/foo => 404', async () => { + const server = createServer({ basePath: '', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/bar/foo' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/ => /', async () => { + const server = createServer({ basePath: '', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/' + }); + + expect(resp.statusCode).to.be(200); + expect(resp.payload).to.be('resp:/'); + }); + + it('/foo => /foo', async () => { + const server = createServer({ basePath: '', rewriteBasePath: false }); + 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', () => { + it('/bar => 404', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/bar' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/bar/ => 404', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/bar/' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/bar/foo => 404', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/bar/foo' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/ => /', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: false }); + const resp = await server.inject({ + url: '/' + }); + + expect(resp.statusCode).to.be(200); + expect(resp.payload).to.be('resp:/'); + }); + + it('/foo => /foo', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: false }); + 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', () => { + it('/bar => /', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: true }); + const resp = await server.inject({ + url: '/bar' + }); + + expect(resp.statusCode).to.be(200); + expect(resp.payload).to.be('resp:/'); + }); + + it('/bar/ => 404', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: true }); + 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 server = createServer({ basePath: '/bar', rewriteBasePath: true }); + 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 server = createServer({ basePath: '/bar', rewriteBasePath: true }); + const resp = await server.inject({ + url: '/' + }); + + expect(resp.statusCode).to.be(404); + }); + + it('/foo => 404', async () => { + const server = createServer({ basePath: '/bar', rewriteBasePath: true }); + const resp = await server.inject({ + url: '/foo' + }); + + expect(resp.statusCode).to.be(404); + }); + }); +}); diff --git a/src/server/http/index.js b/src/server/http/index.js index 3c3dfb95656279..cc59f5a00139db 100644 --- a/src/server/http/index.js +++ b/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); diff --git a/src/server/http/setup_base_path_rewrite.js b/src/server/http/setup_base_path_rewrite.js new file mode 100644 index 00000000000000..c0dfbf57e5619b --- /dev/null +++ b/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(); + }); +} diff --git a/src/server/kbn_server.js b/src/server/kbn_server.js index e93666035f5544..028b37ec42e71f 100644 --- a/src/server/kbn_server.js +++ b/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; } From 9053b5e8f35be662165c8254cb6ec647ccbd9ac2 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 16 Feb 2018 13:12:33 -0700 Subject: [PATCH 2/8] [server/rewriteBasePath/docs] end sentences with periods --- config/kibana.yml | 2 +- docs/setup/settings.asciidoc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/kibana.yml b/config/kibana.yml index 33c1bcba2f93fc..3253a66408aa15 100644 --- a/config/kibana.yml +++ b/config/kibana.yml @@ -15,7 +15,7 @@ # 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 +# default to `true` starting in Kibana 7.0. #server.rewriteBasePath: false # The maximum payload size in bytes for incoming server requests. diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 44b5316335ce75..4ba6b719e7ef39 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -85,7 +85,7 @@ The following example shows a valid regionmap configuration. 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. 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.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. From 0a9a64c00d6272f8142186a5438e021e9f8353c1 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 16 Feb 2018 13:13:00 -0700 Subject: [PATCH 3/8] [server/rewriteBasePath] simplify Joi schema a smidge --- src/server/config/schema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/config/schema.js b/src/server/config/schema.js index 49c968249cfc30..a2a4bd6c2431fe 100644 --- a/src/server/config/schema.js +++ b/src/server/config/schema.js @@ -54,7 +54,7 @@ export default () => Joi.object({ 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: Joi.valid(''), + is: '', then: Joi.default(false).valid(false), otherwise: Joi.default(false) }), From ace596d9dfbc479ab8541a01fb493dc18e464fdc Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 16 Feb 2018 13:18:24 -0700 Subject: [PATCH 4/8] [server/rewriteBasePath] rename test file to match source --- .../{base_path_rewrite.js => setup_base_path_rewrite.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/server/http/__tests__/{base_path_rewrite.js => setup_base_path_rewrite.js} (100%) diff --git a/src/server/http/__tests__/base_path_rewrite.js b/src/server/http/__tests__/setup_base_path_rewrite.js similarity index 100% rename from src/server/http/__tests__/base_path_rewrite.js rename to src/server/http/__tests__/setup_base_path_rewrite.js From 49e642c7ddfd77aa67fdeff2245286d2837f1d2f Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 16 Feb 2018 13:19:08 -0700 Subject: [PATCH 5/8] [server/rewriteBasePath] initialize server in before/after hooks --- .../http/__tests__/setup_base_path_rewrite.js | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/server/http/__tests__/setup_base_path_rewrite.js b/src/server/http/__tests__/setup_base_path_rewrite.js index e329e495a80151..d15aa21096ee13 100644 --- a/src/server/http/__tests__/setup_base_path_rewrite.js +++ b/src/server/http/__tests__/setup_base_path_rewrite.js @@ -4,7 +4,7 @@ import sinon from 'sinon'; import { setupBasePathRewrite } from '../setup_base_path_rewrite'; -describe('server / base path rewriting', () => { +describe('server / setup_base_path_rewrite', () => { function createServer({ basePath, rewriteBasePath }) { const config = { get: sinon.stub() @@ -39,8 +39,11 @@ describe('server / base path rewriting', () => { } describe('no base path', () => { + let server; + before(() => server = createServer({ basePath: '', rewriteBasePath: false })); + after(() => server = undefined); + it('/bar => 404', async () => { - const server = createServer({ basePath: '', rewriteBasePath: false }); const resp = await server.inject({ url: '/bar' }); @@ -49,7 +52,6 @@ describe('server / base path rewriting', () => { }); it('/bar/ => 404', async () => { - const server = createServer({ basePath: '', rewriteBasePath: false }); const resp = await server.inject({ url: '/bar/' }); @@ -58,7 +60,6 @@ describe('server / base path rewriting', () => { }); it('/bar/foo => 404', async () => { - const server = createServer({ basePath: '', rewriteBasePath: false }); const resp = await server.inject({ url: '/bar/foo' }); @@ -67,7 +68,6 @@ describe('server / base path rewriting', () => { }); it('/ => /', async () => { - const server = createServer({ basePath: '', rewriteBasePath: false }); const resp = await server.inject({ url: '/' }); @@ -77,7 +77,6 @@ describe('server / base path rewriting', () => { }); it('/foo => /foo', async () => { - const server = createServer({ basePath: '', rewriteBasePath: false }); const resp = await server.inject({ url: '/foo' }); @@ -88,8 +87,11 @@ describe('server / base path rewriting', () => { }); describe('base path /bar, rewrite = false', () => { + let server; + before(() => server = createServer({ basePath: '/bar', rewriteBasePath: false })); + after(() => server = undefined); + it('/bar => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: false }); const resp = await server.inject({ url: '/bar' }); @@ -98,7 +100,6 @@ describe('server / base path rewriting', () => { }); it('/bar/ => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: false }); const resp = await server.inject({ url: '/bar/' }); @@ -107,7 +108,6 @@ describe('server / base path rewriting', () => { }); it('/bar/foo => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: false }); const resp = await server.inject({ url: '/bar/foo' }); @@ -116,7 +116,6 @@ describe('server / base path rewriting', () => { }); it('/ => /', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: false }); const resp = await server.inject({ url: '/' }); @@ -126,7 +125,6 @@ describe('server / base path rewriting', () => { }); it('/foo => /foo', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: false }); const resp = await server.inject({ url: '/foo' }); @@ -137,8 +135,11 @@ describe('server / base path rewriting', () => { }); describe('base path /bar, rewrite = true', () => { + let server; + before(() => server = createServer({ basePath: '/bar', rewriteBasePath: true })); + after(() => server = undefined); + it('/bar => /', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: true }); const resp = await server.inject({ url: '/bar' }); @@ -148,7 +149,6 @@ describe('server / base path rewriting', () => { }); it('/bar/ => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: true }); const resp = await server.inject({ url: '/bar/' }); @@ -158,7 +158,6 @@ describe('server / base path rewriting', () => { }); it('/bar/foo => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: true }); const resp = await server.inject({ url: '/bar/foo' }); @@ -168,7 +167,6 @@ describe('server / base path rewriting', () => { }); it('/ => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: true }); const resp = await server.inject({ url: '/' }); @@ -177,7 +175,6 @@ describe('server / base path rewriting', () => { }); it('/foo => 404', async () => { - const server = createServer({ basePath: '/bar', rewriteBasePath: true }); const resp = await server.inject({ url: '/foo' }); From d845c62dc6ed4b065ffda4384c4c906f560de7fe Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 19 Feb 2018 11:00:33 -0700 Subject: [PATCH 6/8] [server/rewriteBasePath] rephrase deprecation warning --- src/server/config/transform_deprecations.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/server/config/transform_deprecations.js b/src/server/config/transform_deprecations.js index 959a4c7de59866..ecbe4da3976bc8 100644 --- a/src/server/config/transform_deprecations.js +++ b/src/server/config/transform_deprecations.js @@ -28,10 +28,9 @@ const savedObjectsIndexCheckTimeout = (settings, log) => { const rewriteBasePath = (settings, log) => { if (_.has(settings, 'server.basePath') && !_.has(settings, 'server.rewriteBasePath')) { log( - 'Setting server.basePath without server.rewriteBasePath is deprecated until Kibana 7.0, ' + - 'when the default behavior of server.basePath will change and Kibana will start expecting ' + - 'that all requests start with the server.basePath rather expecting you to rewrite the ' + - 'requests in your reverse proxy. Set server.rewriteBasePath to false to preserve the ' + + '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.' ); } From d16811972e4036e4604c632086855c1b6da856dc Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 19 Feb 2018 11:02:25 -0700 Subject: [PATCH 7/8] [server/config/schema] verify that non-strings are not accepted for basePath --- src/server/config/__tests__/schema.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/server/config/__tests__/schema.js b/src/server/config/__tests__/schema.js index a73e425f114bce..019eeb774b64a8 100644 --- a/src/server/config/__tests__/schema.js +++ b/src/server/config/__tests__/schema.js @@ -41,6 +41,14 @@ describe('Config schema', function () { expect(error).to.have.property('details'); 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 () { From b93d1ed8a14c6bcbee10647515f58a91661e9637 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 19 Feb 2018 11:02:59 -0700 Subject: [PATCH 8/8] [server/config/schema] toss a trailing comma in there --- src/server/config/schema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/config/schema.js b/src/server/config/schema.js index a2a4bd6c2431fe..3c3d37664ee248 100644 --- a/src/server/config/schema.js +++ b/src/server/config/schema.js @@ -56,7 +56,7 @@ export default () => Joi.object({ rewriteBasePath: Joi.boolean().when('basePath', { is: '', then: Joi.default(false).valid(false), - otherwise: Joi.default(false) + otherwise: Joi.default(false), }), customResponseHeaders: Joi.object().unknown(true).default({}), ssl: Joi.object({