From 5ad744f7de4946b091dc27b38a391a27169e41d0 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 3 Jan 2020 16:57:34 -0700 Subject: [PATCH 1/6] [Reporting] Update some runtime validations --- docs/settings/reporting-settings.asciidoc | 8 +++++ x-pack/legacy/plugins/reporting/config.ts | 2 +- ...e_config.js => validate_encryption_key.js} | 7 ++--- .../__tests__/validate_server_host.ts | 30 +++++++++++++++++++ .../reporting/server/lib/validate/index.ts | 10 +++---- ...e_config.ts => validate_encryption_key.ts} | 3 +- .../validate/validate_max_content_length.ts | 1 + .../lib/validate/validate_server_host.ts | 27 +++++++++++++++++ 8 files changed, 77 insertions(+), 11 deletions(-) rename x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/{validate_config.js => validate_encryption_key.js} (79%) create mode 100644 x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_server_host.ts rename x-pack/legacy/plugins/reporting/server/lib/validate/{validate_config.ts => validate_encryption_key.ts} (81%) create mode 100644 x-pack/legacy/plugins/reporting/server/lib/validate/validate_server_host.ts diff --git a/docs/settings/reporting-settings.asciidoc b/docs/settings/reporting-settings.asciidoc index a754f91e9f22a..a9fa2bd18d315 100644 --- a/docs/settings/reporting-settings.asciidoc +++ b/docs/settings/reporting-settings.asciidoc @@ -54,6 +54,14 @@ The protocol for accessing Kibana, typically `http` or `https`. `xpack.reporting.kibanaServer.hostname`:: The hostname for accessing {kib}, if different from the `server.host` value. +[NOTE] +============ +Reporting authenticates requests on the Kibana page only when the hostname matches the +`xpack.reporting.kibanaServer.hostname` setting. Therefore Reporting would fail if the +set value redirects to another server. For that reason, `"0"` is an invalid setting +because, in the Reporting browser, it becomes an automatic redirect to `"0.0.0.0"`. +============ + [float] [[reporting-job-queue-settings]] diff --git a/x-pack/legacy/plugins/reporting/config.ts b/x-pack/legacy/plugins/reporting/config.ts index 8b1d6f6f19805..34fc1f452fbc0 100644 --- a/x-pack/legacy/plugins/reporting/config.ts +++ b/x-pack/legacy/plugins/reporting/config.ts @@ -14,7 +14,7 @@ export async function config(Joi: any) { enabled: Joi.boolean().default(true), kibanaServer: Joi.object({ protocol: Joi.string().valid(['http', 'https']), - hostname: Joi.string(), + hostname: Joi.string().invalid('0'), port: Joi.number().integer(), }).default(), queue: Joi.object({ diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_config.js b/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js similarity index 79% rename from x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_config.js rename to x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js index 8b5d6f4591ff5..37584e2ca46fc 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_config.js +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js @@ -6,10 +6,9 @@ import expect from '@kbn/expect'; import sinon from 'sinon'; -import { validateConfig } from '../validate_config'; +import { validateEncryptionKey } from '../validate_encryption_key'; -// FAILING: https://github.com/elastic/kibana/issues/51373 -describe.skip('Reporting: Validate config', () => { +describe('Reporting: Validate config', () => { const logger = { warning: sinon.spy(), }; @@ -25,7 +24,7 @@ describe.skip('Reporting: Validate config', () => { set: sinon.stub(), }; - expect(() => validateConfig(config, logger)).not.to.throwError(); + expect(() => validateEncryptionKey(config, logger)).not.to.throwError(); sinon.assert.calledWith(config.set, 'xpack.reporting.encryptionKey'); sinon.assert.calledWithMatch(logger.warning, /Generating a random key/); diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_server_host.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_server_host.ts new file mode 100644 index 0000000000000..04f998fd3e5a5 --- /dev/null +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_server_host.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; +import sinon from 'sinon'; +import { ServerFacade } from '../../../../types'; +import { validateServerHost } from '../validate_server_host'; + +const configKey = 'xpack.reporting.kibanaServer.hostname'; + +describe('Reporting: Validate server host setting', () => { + it(`should log a warning and set ${configKey} if server.host is "0"`, () => { + const getStub = sinon.stub(); + getStub.withArgs('server.host').returns('0'); + getStub.withArgs(configKey).returns(undefined); + const config = { + get: getStub, + set: sinon.stub(), + }; + + expect(() => + validateServerHost(({ config: () => config } as unknown) as ServerFacade) + ).to.throwError(); + + sinon.assert.calledWith(config.set, configKey); + }); +}); diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts index 672f90358aba4..74f7118bdf0fb 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts @@ -7,8 +7,9 @@ import { ServerFacade, Logger } from '../../../types'; import { HeadlessChromiumDriverFactory } from '../../browsers/chromium/driver_factory'; import { validateBrowser } from './validate_browser'; -import { validateConfig } from './validate_config'; +import { validateEncryptionKey } from './validate_encryption_key'; import { validateMaxContentLength } from './validate_max_content_length'; +import { validateServerHost } from './validate_server_host'; export async function runValidations( server: ServerFacade, @@ -18,13 +19,12 @@ export async function runValidations( try { await Promise.all([ validateBrowser(server, browserFactory, logger), - validateConfig(server, logger), + validateEncryptionKey(server, logger), validateMaxContentLength(server, logger), + validateServerHost(server), ]); logger.debug(`Reporting plugin self-check ok!`); } catch (err) { - logger.warning( - `Reporting plugin self-check failed. Please check the Kibana Reporting settings. ${err}` - ); + logger.warning(`Reporting plugin self-check generated a warning: ${err}`); } } diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_config.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts similarity index 81% rename from x-pack/legacy/plugins/reporting/server/lib/validate/validate_config.ts rename to x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts index a1eb7be6ecae4..3c821c411b098 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_config.ts +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts @@ -7,11 +7,12 @@ import crypto from 'crypto'; import { ServerFacade, Logger } from '../../../types'; -export function validateConfig(serverFacade: ServerFacade, logger: Logger) { +export function validateEncryptionKey(serverFacade: ServerFacade, logger: Logger) { const config = serverFacade.config(); const encryptionKey = config.get('xpack.reporting.encryptionKey'); if (encryptionKey == null) { + // TODO this should simply throw an error and let the handler conver it to a warning mesasge. See validateServerHost. logger.warning( `Generating a random key for xpack.reporting.encryptionKey. To prevent pending reports from failing on restart, please set ` + `xpack.reporting.encryptionKey in kibana.yml` diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_max_content_length.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_max_content_length.ts index ca38ce5d635c6..f91cd40bfd3c7 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_max_content_length.ts +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_max_content_length.ts @@ -25,6 +25,7 @@ export async function validateMaxContentLength(server: ServerFacade, logger: Log const kibanaMaxContentBytes: number = config.get(KIBANA_MAX_SIZE_BYTES_PATH); if (kibanaMaxContentBytes > elasticSearchMaxContentBytes) { + // TODO this should simply throw an error and let the handler conver it to a warning mesasge. See validateServerHost. logger.warning( `${KIBANA_MAX_SIZE_BYTES_PATH} (${kibanaMaxContentBytes}) is higher than ElasticSearch's ${ES_MAX_SIZE_BYTES_PATH} (${elasticSearchMaxContentBytes}). ` + `Please set ${ES_MAX_SIZE_BYTES_PATH} in ElasticSearch to match, or lower your ${KIBANA_MAX_SIZE_BYTES_PATH} in Kibana to avoid this warning.` diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_server_host.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_server_host.ts new file mode 100644 index 0000000000000..f4f4d61246b6a --- /dev/null +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_server_host.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { ServerFacade } from '../../../types'; + +const configKey = 'xpack.reporting.kibanaServer.hostname'; + +export function validateServerHost(serverFacade: ServerFacade) { + const config = serverFacade.config(); + + const serverHost = config.get('server.host'); + const reportingKibanaHostName = config.get(configKey); + + if (!reportingKibanaHostName && serverHost === '0') { + // @ts-ignore: No set() method on KibanaConfig, just get() and has() + config.set(configKey, '0.0.0.0'); // update config in memory to allow Reporting to work + + throw new Error( + `Found 'server.host: "0"' in settings. This is incompatible with Reporting. ` + + `To enable Reporting to work, '${configKey}: 0.0.0.0' is being automatically to the configuration. ` + + `You can change to 'server.host: 0.0.0.0' or add '${configKey}: 0.0.0.0' in kibana.yml to prevent this message.` + ); + } +} From 7bf9f8bc3c59efbba287c5552b53d915e8bf0239 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 6 Jan 2020 17:48:42 -0700 Subject: [PATCH 2/6] fix unit test --- .../server/lib/validate/__tests__/validate_encryption_key.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js b/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js index 37584e2ca46fc..10980f702d849 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/__tests__/validate_encryption_key.js @@ -24,7 +24,7 @@ describe('Reporting: Validate config', () => { set: sinon.stub(), }; - expect(() => validateEncryptionKey(config, logger)).not.to.throwError(); + expect(() => validateEncryptionKey({ config: () => config }, logger)).not.to.throwError(); sinon.assert.calledWith(config.set, 'xpack.reporting.encryptionKey'); sinon.assert.calledWithMatch(logger.warning, /Generating a random key/); From 2d03066f9fee6292e0871242bf11947485eb9fbe Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 7 Jan 2020 15:21:20 -0700 Subject: [PATCH 3/6] i18n --- .../reporting/server/lib/validate/index.ts | 16 ++++++++++++++-- .../lib/validate/validate_encryption_key.ts | 11 +++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts index 74f7118bdf0fb..79a64bd82d022 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/index.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; import { ServerFacade, Logger } from '../../../types'; import { HeadlessChromiumDriverFactory } from '../../browsers/chromium/driver_factory'; import { validateBrowser } from './validate_browser'; @@ -23,8 +24,19 @@ export async function runValidations( validateMaxContentLength(server, logger), validateServerHost(server), ]); - logger.debug(`Reporting plugin self-check ok!`); + logger.debug( + i18n.translate('xpack.reporting.selfCheck.ok', { + defaultMessage: `Reporting plugin self-check ok!`, + }) + ); } catch (err) { - logger.warning(`Reporting plugin self-check generated a warning: ${err}`); + logger.warning( + i18n.translate('xpack.reporting.selfCheck.warning', { + defaultMessage: `Reporting plugin self-check generated a warning: {err}`, + values: { + err, + }, + }) + ); } } diff --git a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts index 3c821c411b098..e0af94cbdc29c 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts +++ b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; import crypto from 'crypto'; import { ServerFacade, Logger } from '../../../types'; @@ -14,8 +15,14 @@ export function validateEncryptionKey(serverFacade: ServerFacade, logger: Logger if (encryptionKey == null) { // TODO this should simply throw an error and let the handler conver it to a warning mesasge. See validateServerHost. logger.warning( - `Generating a random key for xpack.reporting.encryptionKey. To prevent pending reports from failing on restart, please set ` + - `xpack.reporting.encryptionKey in kibana.yml` + i18n.translate('xpack.reporting.selfCheckEncryptionKey.warning', { + defaultMessage: + `Generating a random key for {setting}. To prevent pending reports ` + + `from failing on restart, please set {setting} in kibana.yml`, + values: { + setting: 'xpack.reporting.encryptionKey', + }, + }) ); // @ts-ignore: No set() method on KibanaConfig, just get() and has() From 4c8595367dd11fc0317bbc7439cabbc68716e3cf Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 7 Jan 2020 15:21:45 -0700 Subject: [PATCH 4/6] make warning logging of encryptionKey possible --- x-pack/legacy/plugins/reporting/config.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/reporting/config.ts b/x-pack/legacy/plugins/reporting/config.ts index 34fc1f452fbc0..68185b29ff2f7 100644 --- a/x-pack/legacy/plugins/reporting/config.ts +++ b/x-pack/legacy/plugins/reporting/config.ts @@ -140,7 +140,9 @@ export async function config(Joi: any) { }).default(), encryptionKey: Joi.when(Joi.ref('$dist'), { is: true, - then: Joi.string(), + then: Joi.string() + .allow(null) + .default(null), otherwise: Joi.string().default('a'.repeat(32)), }), roles: Joi.object({ From d91973ce945473dbba940107dbf15a97fdf33823 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 8 Jan 2020 13:25:13 -0700 Subject: [PATCH 5/6] update snapshot --- .../legacy/plugins/reporting/__snapshots__/index.test.js.snap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap b/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap index 469f5e6e7b3c6..42fa20bf3516c 100644 --- a/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap +++ b/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap @@ -154,6 +154,7 @@ Object { }, }, "enabled": true, + "encryptionKey": null, "index": ".reporting", "kibanaServer": Object {}, "poll": Object { @@ -335,6 +336,7 @@ Object { }, }, "enabled": true, + "encryptionKey": null, "index": ".reporting", "kibanaServer": Object {}, "poll": Object { From 17f60409521efda35a170c306edbb205bac60293 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 8 Jan 2020 17:04:39 -0700 Subject: [PATCH 6/6] revert unrelated config change --- .../legacy/plugins/reporting/__snapshots__/index.test.js.snap | 2 -- x-pack/legacy/plugins/reporting/config.ts | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap b/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap index 42fa20bf3516c..469f5e6e7b3c6 100644 --- a/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap +++ b/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap @@ -154,7 +154,6 @@ Object { }, }, "enabled": true, - "encryptionKey": null, "index": ".reporting", "kibanaServer": Object {}, "poll": Object { @@ -336,7 +335,6 @@ Object { }, }, "enabled": true, - "encryptionKey": null, "index": ".reporting", "kibanaServer": Object {}, "poll": Object { diff --git a/x-pack/legacy/plugins/reporting/config.ts b/x-pack/legacy/plugins/reporting/config.ts index 68185b29ff2f7..34fc1f452fbc0 100644 --- a/x-pack/legacy/plugins/reporting/config.ts +++ b/x-pack/legacy/plugins/reporting/config.ts @@ -140,9 +140,7 @@ export async function config(Joi: any) { }).default(), encryptionKey: Joi.when(Joi.ref('$dist'), { is: true, - then: Joi.string() - .allow(null) - .default(null), + then: Joi.string(), otherwise: Joi.string().default('a'.repeat(32)), }), roles: Joi.object({