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

Configurable headers for all elasticsearch requests #7996

Merged
merged 1 commit into from
Aug 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/kibana.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@
# headers, set this value to [] (an empty list).
# elasticsearch.requestHeadersWhitelist: [ authorization ]

# Header names and values that are sent to Elasticsearch. Any custom headers cannot be overwritten
# by client-side headers, regardless of the elasticsearch.requestHeadersWhitelist configuration.
# elasticsearch.customHeaders: {}

# Time in milliseconds for Elasticsearch to wait for responses from shards. Set to 0 to disable.
# elasticsearch.shardTimeout: 0

Expand Down
2 changes: 2 additions & 0 deletions docs/kibana-yml.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ wait for Elasticsearch to respond to pings.
Elasticsearch. This value must be a positive integer.
`elasticsearch.requestHeadersWhitelist:`:: *Default: `[ 'authorization' ]`* List of Kibana client-side headers to send to Elasticsearch.
To send *no* client-side headers, set this value to [] (an empty list).
`elasticsearch.customHeaders:`:: *Default: `{}`* Header names and values to send to Elasticsearch. Any custom headers
cannot be overwritten by client-side headers, regardless of the `elasticsearch.requestHeadersWhitelist` configuration.
`elasticsearch.shardTimeout:`:: *Default: 0* Time in milliseconds for Elasticsearch to wait for responses from shards. Set
to 0 to disable.
`elasticsearch.startupTimeout:`:: *Default: 5000* Time in milliseconds to wait for Elasticsearch at Kibana startup before
Expand Down
1 change: 1 addition & 0 deletions src/core_plugins/elasticsearch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module.exports = function ({ Plugin }) {
shardTimeout: number().default(0),
requestTimeout: number().default(30000),
requestHeadersWhitelist: array().items().single().default(DEFAULT_REQUEST_HEADERS),
customHeaders: object().default({}),
pingTimeout: number().default(ref('requestTimeout')),
startupTimeout: number().default(5000),
ssl: object({
Expand Down
64 changes: 38 additions & 26 deletions src/core_plugins/elasticsearch/lib/__tests__/map_uri.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,35 @@ describe('plugins/elasticsearch', function () {
};
});

it('only sends the whitelisted request headers', function () {
it('sends custom headers if set', function () {
const get = sinon.stub();
get.withArgs('elasticsearch.requestHeadersWhitelist').returns([]);
get.withArgs('elasticsearch.customHeaders').returns({ foo: 'bar' });
const server = { config: () => ({ get }) };

const get = sinon.stub()
.withArgs('elasticsearch.url').returns('http://foobar:9200')
.withArgs('elasticsearch.requestHeadersWhitelist').returns(['x-my-custom-HEADER', 'Authorization']);
const config = function () { return { get: get }; };
const server = {
config: config
};
mapUri(server)(request, function (err, upstreamUri, upstreamHeaders) {
expect(err).to.be(null);
expect(upstreamHeaders).to.have.property('foo', 'bar');
});
});

it('sends configured custom headers even if the same named header exists in request', function () {
const get = sinon.stub();
get.withArgs('elasticsearch.requestHeadersWhitelist').returns(['x-my-custom-header']);
get.withArgs('elasticsearch.customHeaders').returns({'x-my-custom-header': 'asconfigured'});
const server = { config: () => ({ get }) };

mapUri(server)(request, function (err, upstreamUri, upstreamHeaders) {
expect(err).to.be(null);
expect(upstreamHeaders).to.have.property('x-my-custom-header', 'asconfigured');
});
});

it('only proxies the whitelisted request headers', function () {
const get = sinon.stub();
get.withArgs('elasticsearch.requestHeadersWhitelist').returns(['x-my-custom-HEADER', 'Authorization']);
get.withArgs('elasticsearch.customHeaders').returns({});
const server = { config: () => ({ get }) };

mapUri(server)(request, function (err, upstreamUri, upstreamHeaders) {
expect(err).to.be(null);
Expand All @@ -40,31 +60,23 @@ describe('plugins/elasticsearch', function () {
});
});

it('sends no headers if whitelist is set to []', function () {

const get = sinon.stub()
.withArgs('elasticsearch.url').returns('http://foobar:9200')
.withArgs('elasticsearch.requestHeadersWhitelist').returns([]);
const config = function () { return { get: get }; };
const server = {
config: config
};
it('proxies no headers if whitelist is set to []', function () {
const get = sinon.stub();
get.withArgs('elasticsearch.requestHeadersWhitelist').returns([]);
get.withArgs('elasticsearch.customHeaders').returns({});
const server = { config: () => ({ get }) };

mapUri(server)(request, function (err, upstreamUri, upstreamHeaders) {
expect(err).to.be(null);
expect(Object.keys(upstreamHeaders).length).to.be(0);
});
});

it('sends no headers if whitelist is set to no value', function () {

const get = sinon.stub()
.withArgs('elasticsearch.url').returns('http://foobar:9200')
.withArgs('elasticsearch.requestHeadersWhitelist').returns([ null ]); // This is how Joi returns it
const config = function () { return { get: get }; };
const server = {
config: config
};
it('proxies no headers if whitelist is set to no value', function () {
const get = sinon.stub();
get.withArgs('elasticsearch.requestHeadersWhitelist').returns([ null ]); // This is how Joi returns it
get.withArgs('elasticsearch.customHeaders').returns({});
const server = { config: () => ({ get }) };

mapUri(server)(request, function (err, upstreamUri, upstreamHeaders) {
expect(err).to.be(null);
Expand Down
39 changes: 39 additions & 0 deletions src/core_plugins/elasticsearch/lib/__tests__/set_headers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import expect from 'expect.js';
import sinon from 'sinon';
import setHeaders from '../set_headers';

describe('plugins/elasticsearch', function () {
describe('lib/set_headers', function () {
it('throws if not given an object as the first argument', function () {
const fn = () => setHeaders(null, {});
expect(fn).to.throwError();
});

it('throws if not given an object as the second argument', function () {
const fn = () => setHeaders({}, null);
expect(fn).to.throwError();
});

it('returns a new object', function () {
const originalHeaders = {};
const newHeaders = {};
const returnedHeaders = setHeaders(originalHeaders, newHeaders);
expect(returnedHeaders).not.to.be(originalHeaders);
expect(returnedHeaders).not.to.be(newHeaders);
});

it('returns object with newHeaders merged with originalHeaders', function () {
const originalHeaders = { foo: 'bar' };
const newHeaders = { one: 'two' };
const returnedHeaders = setHeaders(originalHeaders, newHeaders);
expect(returnedHeaders).to.eql({ foo: 'bar', one: 'two' });
});

it('returns object where newHeaders takes precedence for any matching keys', function () {
const originalHeaders = { foo: 'bar' };
const newHeaders = { one: 'two', foo: 'notbar' };
const returnedHeaders = setHeaders(originalHeaders, newHeaders);
expect(returnedHeaders).to.eql({ foo: 'notbar', one: 'two' });
});
});
});
14 changes: 12 additions & 2 deletions src/core_plugins/elasticsearch/lib/expose_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,19 @@ module.exports = function (server) {
ssl.ca = options.ca.map(readFile);
}

const host = {
host: uri.hostname,
port: uri.port,
protocol: uri.protocol,
path: uri.pathname,
auth: uri.auth,
query: uri.query,
headers: config.get('elasticsearch.customHeaders')
};

return new elasticsearch.Client({
host: url.format(uri),
ssl: ssl,
host,
ssl,
plugins: options.plugins,
apiVersion: options.apiVersion,
keepAlive: options.keepAlive,
Expand Down
8 changes: 5 additions & 3 deletions src/core_plugins/elasticsearch/lib/map_uri.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import querystring from 'querystring';
import { resolve } from 'url';
import filterHeaders from './filter_headers';
import setHeaders from './set_headers';

module.exports = function mapUri(server, prefix) {
export default function mapUri(server, prefix) {

const config = server.config();
return function (request, done) {
Expand All @@ -14,7 +15,8 @@ module.exports = function mapUri(server, prefix) {
}
const query = querystring.stringify(request.query);
if (query) url += '?' + query;
const filteredHeaders = filterHeaders(request.headers, server.config().get('elasticsearch.requestHeadersWhitelist'));
done(null, url, filteredHeaders);
const filteredHeaders = filterHeaders(request.headers, config.get('elasticsearch.requestHeadersWhitelist'));
const customHeaders = setHeaders(filteredHeaders, config.get('elasticsearch.customHeaders'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly curious: what was your rationale behind creating setHeaders as opposed to using something like _.merge here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the object checks and such, it seemed like a bit too much logic to dump directly into map_uri, which is already juggling a couple of responsibilities.

done(null, url, customHeaders);
};
};
15 changes: 15 additions & 0 deletions src/core_plugins/elasticsearch/lib/set_headers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { isPlainObject } from 'lodash';

export default function setHeaders(originalHeaders, newHeaders) {
if (!isPlainObject(originalHeaders)) {
throw new Error(`Expected originalHeaders to be an object, but ${typeof originalHeaders} given`);
}
if (!isPlainObject(newHeaders)) {
throw new Error(`Expected newHeaders to be an object, but ${typeof newHeaders} given`);
}

return {
...originalHeaders,
...newHeaders
};
}