From a5867b531d2b2b5214182c277f5826bdd8ef3175 Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Thu, 9 Jan 2020 10:13:22 -0700 Subject: [PATCH] [Reporting] Update some runtime validations (#53975) * [Reporting] Update some runtime validations * fix unit test * i18n * make warning logging of encryptionKey possible * update snapshot * revert unrelated config change --- 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 | 20 ++++++++++--- ...e_config.ts => validate_encryption_key.ts} | 14 +++++++-- .../validate/validate_max_content_length.ts | 1 + .../lib/validate/validate_server_host.ts | 27 +++++++++++++++++ 8 files changed, 97 insertions(+), 12 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} (55%) 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..10980f702d849 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: () => 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..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,11 +4,13 @@ * 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'; -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 +20,23 @@ 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!`); + logger.debug( + i18n.translate('xpack.reporting.selfCheck.ok', { + defaultMessage: `Reporting plugin self-check ok!`, + }) + ); } catch (err) { logger.warning( - `Reporting plugin self-check failed. Please check the Kibana Reporting settings. ${err}` + 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_config.ts b/x-pack/legacy/plugins/reporting/server/lib/validate/validate_encryption_key.ts similarity index 55% 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..e0af94cbdc29c 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 @@ -4,17 +4,25 @@ * 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'; -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` + 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() 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.` + ); + } +}