From fac1a917d6854a8cb00caa58e3d27e78f0c452d4 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Wed, 4 Feb 2015 13:21:36 -0700 Subject: [PATCH 1/3] Refactor Proxy Authentication and Error Handling Closes #2872 Added check if the headers have been sent before sending the error message to the browser. --- src/server/bin/kibana.js | 12 +++++++++++- src/server/config/kibana.yml | 14 +++++++++----- src/server/index.js | 1 - src/server/lib/elasticsearch_client.js | 12 ++++++++++++ src/server/lib/migrateConfig.js | 11 +---------- src/server/lib/upgradeConfig.js | 6 +----- src/server/routes/proxy.js | 12 +++--------- 7 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 src/server/lib/elasticsearch_client.js diff --git a/src/server/bin/kibana.js b/src/server/bin/kibana.js index 6a68d74f07940..048080a727a3a 100755 --- a/src/server/bin/kibana.js +++ b/src/server/bin/kibana.js @@ -60,11 +60,21 @@ var server = require('../'); var logger = require('../lib/logger'); server.start(function (err) { if (!err && config.kibana.pid_file) { - fs.writeFile(config.kibana.pid_file, process.pid, function (err) { + return fs.writeFile(config.kibana.pid_file, process.pid, function (err) { if (err) { logger.fatal('Failed to write PID file to %s', config.kibana.pid_file); process.exit(1); } }); } + + // If we get here then things have gone sideways. Let's wait 2 milliseconds + // for the logger to flush before exiting. + if (err) { + logger.fatal({ err: err }); + setTimeout(function () { + process.exit(1); + }, 2); + } + }); diff --git a/src/server/config/kibana.yml b/src/server/config/kibana.yml index ba654c50eaad6..3bc7102bf164d 100644 --- a/src/server/config/kibana.yml +++ b/src/server/config/kibana.yml @@ -7,10 +7,6 @@ host: "0.0.0.0" # The Elasticsearch instance to use for all your queries. elasticsearch_url: "http://localhost:9200" -# If your Elasticsearch is protected with basic auth: -# elasticsearch_username: user -# elasticsearch_password: pass - # preserve_elasticsearch_host true will send the hostname specified in `elasticsearch`. If you set it to false, # then the host you use to connect to *this* Kibana instance will be sent. elasticsearch_preserve_host: true @@ -19,12 +15,20 @@ elasticsearch_preserve_host: true # and dashboards. It will create a new index if it doesn't already exist. kibana_index: ".kibana" +# If your Elasticsearch is protected with basic auth, this is the user credentials +# used by the Kibana server to perform maintence on the kibana_index at statup. Your Kibana +# users will still need to authenticate with Elasticsearch (which is proxied thorugh +# the Kibana server) +# kibana_elasticsearch_username: user +# kibana_elasticsearch_password: pass + + # The default application to load. default_app_id: "discover" # Time in milliseconds to wait for responses from the back end or elasticsearch. # This must be > 0 -request_timeout: 500000 +request_timeout: 300000 # Time in milliseconds for Elasticsearch to wait for responses from shards. # Set to 0 to disable. diff --git a/src/server/index.js b/src/server/index.js index fa4d3104cc66e..d4451ebc12efd 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -55,7 +55,6 @@ function onError(error) { process.exit(1); break; default: - logger.error({ err: error }); throw error; } } diff --git a/src/server/lib/elasticsearch_client.js b/src/server/lib/elasticsearch_client.js new file mode 100644 index 0000000000000..b4e1a0b3cac58 --- /dev/null +++ b/src/server/lib/elasticsearch_client.js @@ -0,0 +1,12 @@ +var config = require('../config'); +var elasticsearch = require('elasticsearch'); +var util = require('util'); +var url = require('url'); +var uri = url.parse(config.elasticsearch); +if (config.kibana.kibana_elasticsearch_username && config.kibana.kibana_elasticsearch_password) { + uri.auth = util.format('%s:%s', config.kibana.kibana_elasticsearch_username, config.kibana.kibana_elasticsearch_password); +} +module.exports = new elasticsearch.Client({ + host: url.format(uri) +}); + diff --git a/src/server/lib/migrateConfig.js b/src/server/lib/migrateConfig.js index 0e07c9bf74921..7000d9d3c6c06 100644 --- a/src/server/lib/migrateConfig.js +++ b/src/server/lib/migrateConfig.js @@ -1,15 +1,6 @@ var config = require('../config'); -var elasticsearch = require('elasticsearch'); var upgrade = require('./upgradeConfig'); -var util = require('util'); -var url = require('url'); -var uri = url.parse(config.elasticsearch); -if (config.kibana.elasticsearch_username && config.kibana.elasticsearch_password) { - uri.auth = util.format('%s:%s', config.kibana.elasticsearch_username, config.kibana.elasticsearch_password); -} -var client = new elasticsearch.Client({ - host: url.format(uri) -}); +var client = require('./elasticsearch_client'); module.exports = function () { var options = { diff --git a/src/server/lib/upgradeConfig.js b/src/server/lib/upgradeConfig.js index b6e9730a92cd6..1a325dfd413ec 100644 --- a/src/server/lib/upgradeConfig.js +++ b/src/server/lib/upgradeConfig.js @@ -2,11 +2,7 @@ var Promise = require('bluebird'); var isUpgradeable = require('./isUpgradeable'); var config = require('../config'); var _ = require('lodash'); -var elasticsearch = require('elasticsearch'); -var client = new elasticsearch.Client({ - host: config.elasticsearch -}); - +var client = require('./elasticsearch_client'); module.exports = function (response) { var newConfig = {}; // Check to see if there are any doc. If not then we can assume diff --git a/src/server/routes/proxy.js b/src/server/routes/proxy.js index cacdc53672f50..5c6c6d220d3ed 100644 --- a/src/server/routes/proxy.js +++ b/src/server/routes/proxy.js @@ -8,6 +8,7 @@ var fs = require('fs'); var url = require('url'); var target = url.parse(config.elasticsearch); var join = require('path').join; +var logger = require('../lib/logger'); // If the target is backed by an SSL and a CA is provided via the config @@ -48,7 +49,6 @@ router.use(function (req, res, next) { timeout: config.kibana.request_timeout }; - options.headers['x-forward-for'] = req.connection.remoteAddress || req.socket.remoteAddress; options.headers['x-forward-port'] = getPort(req); options.headers['x-forward-proto'] = req.connection.pair ? 'https' : 'http'; @@ -63,13 +63,6 @@ router.use(function (req, res, next) { options.body = req.rawBody; } - // Support for handling basic auth - if (config.kibana.elasticsearch_username && config.kibana.elasticsearch_password) { - var code = new buffer.Buffer(config.kibana.elasticsearch_username + ':' + config.kibana.elasticsearch_password); - var auth = 'Basic ' + code.toString('base64'); - options.headers.authorization = auth; - } - // To support the elasticsearch_preserve_host feature we need to change the // host header to the target host header. I don't quite understand the value // of this... but it's a feature we had before so I guess we are keeping it. @@ -80,6 +73,7 @@ router.use(function (req, res, next) { // Create the request and pipe the response var esRequest = request(options); esRequest.on('error', function (err) { + logger.error({ err: err }); var code = 502; var body = { message: 'Bad Gateway' }; @@ -92,7 +86,7 @@ router.use(function (req, res, next) { } body.err = err.message; - res.status(code).json(body); + if (!res.headersSent) res.status(code).json(body); }); esRequest.pipe(res); }); From bb79546e2e51ef6747e5f747ddc50a6d81978caa Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Wed, 4 Feb 2015 13:51:42 -0700 Subject: [PATCH 2/3] Removing some code that made it in unnecessarily --- src/server/bin/kibana.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/server/bin/kibana.js b/src/server/bin/kibana.js index 048080a727a3a..cdb4a654214e7 100755 --- a/src/server/bin/kibana.js +++ b/src/server/bin/kibana.js @@ -68,13 +68,10 @@ server.start(function (err) { }); } - // If we get here then things have gone sideways. Let's wait 2 milliseconds - // for the logger to flush before exiting. + // If we get here then things have gone sideways. if (err) { logger.fatal({ err: err }); - setTimeout(function () { - process.exit(1); - }, 2); + process.exit(1); } }); From aa3d030634c9f4878d9d1f7f60310e2b1c39ee01 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Wed, 4 Feb 2015 13:55:42 -0700 Subject: [PATCH 3/3] Rearranging some code so it's less confusing --- src/server/bin/kibana.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/server/bin/kibana.js b/src/server/bin/kibana.js index cdb4a654214e7..b8f92428836b1 100755 --- a/src/server/bin/kibana.js +++ b/src/server/bin/kibana.js @@ -59,7 +59,13 @@ if (program.host) { var server = require('../'); var logger = require('../lib/logger'); server.start(function (err) { - if (!err && config.kibana.pid_file) { + // If we get here then things have gone sideways and we need to give up. + if (err) { + logger.fatal({ err: err }); + process.exit(1); + } + + if (config.kibana.pid_file) { return fs.writeFile(config.kibana.pid_file, process.pid, function (err) { if (err) { logger.fatal('Failed to write PID file to %s', config.kibana.pid_file); @@ -68,10 +74,4 @@ server.start(function (err) { }); } - // If we get here then things have gone sideways. - if (err) { - logger.fatal({ err: err }); - process.exit(1); - } - });