Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[server/rewriteBasePath] Support rewriting basePath requests #16724

Merged
merged 10 commits into from
Feb 19, 2018
Merged
13 changes: 10 additions & 3 deletions config/kibana.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 2 additions & 3 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

`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.
Expand Down
13 changes: 3 additions & 10 deletions src/cli/cluster/base_path_proxy.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
}
}
});
Expand Down
6 changes: 4 additions & 2 deletions src/cli/cluster/cluster_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for trailing commas

);

serverArgv.push(
`--server.port=${this.basePathProxy.targetPort}`,
`--server.basePath=${this.basePathProxy.basePath}`
`--server.basePath=${this.basePathProxy.basePath}`,
'--server.rewriteBasePath=true',
);
}

Expand Down
45 changes: 43 additions & 2 deletions src/server/config/__tests__/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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');
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a test case rejecting everything but strings for basePath?

});

describe('ssl', function () {
Expand Down
5 changes: 5 additions & 0 deletions src/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
12 changes: 12 additions & 0 deletions src/server/config/transform_deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ const savedObjectsIndexCheckTimeout = (settings, log) => {
}
};

const rewriteBasePath = (settings, log) => {
if (_.has(settings, 'server.basePath') && !_.has(settings, 'server.rewriteBasePath')) {
log(
Copy link
Member

@azasypkin azasypkin Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: is there any magic step (apart from defining deprecated keys in config obviously) I should make to see this warning? I don't see any deprecation warnings defined in this file for some reason :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, well, looks like the server isn't logging deprecation messages... #16795. I think you'll just have to verify the code until @epixa gets to #16795

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, yeah code lgtm

'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'),
Expand All @@ -37,6 +48,7 @@ const deprecations = [
rename('optimize.lazyProxyTimeout', 'optimize.watchProxyTimeout'),
serverSslEnabled,
savedObjectsIndexCheckTimeout,
rewriteBasePath,
];

export const transformDeprecations = createTransform(deprecations);
185 changes: 185 additions & 0 deletions src/server/http/__tests__/setup_base_path_rewrite.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
2 changes: 2 additions & 0 deletions src/server/http/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ 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) {
server = kbnServer.server = new Hapi.Server();

const shortUrlLookup = shortUrlLookupProvider(server);
await kbnServer.mixin(setupConnectionMixin);
await kbnServer.mixin(setupBasePathRewrite);
await kbnServer.mixin(setupRedirectMixin);
await kbnServer.mixin(registerHapiPluginsMixin);

Expand Down
Loading