From 01ce0f7938c7a3ab0fff04d61c70187a58d5874b Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Tue, 25 Jun 2019 14:16:29 -0700 Subject: [PATCH 1/3] Re-working csv injection issue into master --- .../csv/server/__tests__/execute_job.js | 62 ++++++++++++ .../export_types/csv/server/execute_job.js | 3 +- .../lib/check_cells_for_formulas.test.ts | 96 +++++++++++++++++++ .../server/lib/check_cells_for_formulas.ts | 20 ++++ .../csv/server/lib/generate_csv.js | 13 ++- .../public/components/report_listing.tsx | 17 ++++ .../public/hacks/job_completion_notifier.js | 30 +++++- .../reporting/server/lib/esqueue/worker.js | 1 + .../server/routes/lib/get_document_payload.ts | 22 +++++ x-pack/legacy/plugins/reporting/types.d.ts | 1 + 10 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.test.ts create mode 100644 x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js b/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js index 312ddd1661a9e..8729f5fe30f64 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js @@ -296,6 +296,68 @@ describe('CSV Execute Job', function () { }); }); + describe('Cells with formula values', async () => { + it('returns `csv_contains_formulas` when cells contain formulas', async function () { + callWithRequestStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { 'one': '=SUM(A1:A2)', 'two': 'bar' } }] + }, + _scroll_id: 'scrollId' + }); + + const executeJob = executeJobFactory(mockServer); + const jobParams = { + headers: encryptedHeaders, + fields: [ 'one', 'two' ], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }; + const { csv_contains_formulas: csvContainsFormulas } = await executeJob(jobParams, cancellationToken); + + expect(csvContainsFormulas).to.equal(true); + }); + + it('returns warnings when headings contain formulas', async function () { + callWithRequestStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { '=SUM(A1:A2)': 'foo', 'two': 'bar' } }] + }, + _scroll_id: 'scrollId' + }); + + const executeJob = executeJobFactory(mockServer); + const jobParams = { + headers: encryptedHeaders, + fields: [ '=SUM(A1:A2)', 'two' ], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }; + const { csv_contains_formulas: csvContainsFormulas } = await executeJob(jobParams, cancellationToken); + + expect(csvContainsFormulas).to.equal(true); + }); + + it('returns no warnings when cells have no formulas', async function () { + callWithRequestStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { 'one': 'foo', 'two': 'bar' } }] + }, + _scroll_id: 'scrollId' + }); + + const executeJob = executeJobFactory(mockServer); + const jobParams = { + headers: encryptedHeaders, + fields: [ 'one', 'two' ], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }; + const { csv_contains_formulas: csvContainsFormulas } = await executeJob(jobParams, cancellationToken); + + expect(csvContainsFormulas).to.equal(false); + }); + }); + describe('Elasticsearch call errors', function () { it('should reject Promise if search call errors out', async function () { callWithRequestStub.rejects(new Error()); diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js index b0e78b2e4deee..5a130655780ec 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js @@ -92,7 +92,7 @@ function executeJobFn(server) { })(), ]); - const { content, maxSizeReached, size } = await generateCsv({ + const { content, maxSizeReached, size, csvContainsFormulas } = await generateCsv({ searchRequest, fields, metaFields, @@ -112,6 +112,7 @@ function executeJobFn(server) { content, max_size_reached: maxSizeReached, size, + csv_contains_formulas: csvContainsFormulas, }; }; } diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.test.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.test.ts new file mode 100644 index 0000000000000..0107e77bb76ae --- /dev/null +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.test.ts @@ -0,0 +1,96 @@ +/* + * 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 { checkIfRowsHaveFormulas } from './check_cells_for_formulas'; + +const formulaValues = ['=', '+', '-', '@']; +const nonRows = [null, undefined, 9, () => {}]; + +describe(`Check CSV Injected values`, () => { + it(`returns 'false' when there's no formula values in cells`, () => { + expect( + checkIfRowsHaveFormulas( + { + _doc: 'foo-bar', + value: 'cool', + title: 'nice', + }, + ['_doc', 'value', 'title'] + ) + ).toBe(false); + }); + + formulaValues.forEach(formula => { + it(`returns 'true' when cells start with "${formula}"`, () => { + expect( + checkIfRowsHaveFormulas( + { + _doc: 'foo-bar', + value: formula, + title: 'nice', + }, + ['_doc', 'value', 'title'] + ) + ).toBe(true); + }); + + it(`returns 'false' when cells start with "${formula}" but aren't selected`, () => { + expect( + checkIfRowsHaveFormulas( + { + _doc: 'foo-bar', + value: formula, + title: 'nice', + }, + ['_doc', 'title'] + ) + ).toBe(false); + }); + }); + + formulaValues.forEach(formula => { + it(`returns 'true' when headers start with "${formula}"`, () => { + expect( + checkIfRowsHaveFormulas( + { + _doc: 'foo-bar', + [formula]: 'baz', + title: 'nice', + }, + ['_doc', formula, 'title'] + ) + ).toBe(true); + }); + + it(`returns 'false' when headers start with "${formula}" but aren't selected in fields`, () => { + expect( + checkIfRowsHaveFormulas( + { + _doc: 'foo-bar', + [formula]: 'baz', + title: 'nice', + }, + ['_doc', 'title'] + ) + ).toBe(false); + }); + }); + + nonRows.forEach(nonRow => { + it(`returns false when there's "${nonRow}" for rows`, () => { + expect( + checkIfRowsHaveFormulas( + { + _doc: 'foo-bar', + // @ts-ignore need to assert non-string values still return false + value: nonRow, + title: 'nice', + }, + ['_doc', 'value', 'title'] + ) + ).toBe(false); + }); + }); +}); diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts new file mode 100644 index 0000000000000..09f7cd2061ffb --- /dev/null +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts @@ -0,0 +1,20 @@ +/* + * 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 * as _ from 'lodash'; + +const formulaValues = ['=', '+', '-', '@']; + +interface IFlattened { + [header: string]: string; +} + +export const checkIfRowsHaveFormulas = (flattened: IFlattened, fields: string[]) => { + const pruned = _.pick(flattened, fields); + const csvValues = [..._.keys(pruned), ...(_.values(pruned) as string[])]; + + return _.some(csvValues, cell => _.some(formulaValues, char => _.startsWith(cell, char))); +}; diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js index b214c1083058f..8721637c3382b 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js @@ -9,6 +9,7 @@ import { createFormatCsvValues } from './format_csv_values'; import { createEscapeValue } from './escape_value'; import { createHitIterator } from './hit_iterator'; import { MaxSizeStringBuilder } from './max_size_string_builder'; +import { checkIfRowsHaveFormulas } from './check_cells_for_formulas'; export function createGenerateCsv(logger) { const hitIterator = createHitIterator(logger); @@ -35,6 +36,7 @@ export function createGenerateCsv(logger) { const iterator = hitIterator(settings.scroll, callEndpoint, searchRequest, cancellationToken); let maxSizeReached = false; + let csvContainsFormulas = false; const flattenHit = createFlattenHit(fields, metaFields, conflictedTypesFields); const formatCsvValues = createFormatCsvValues(escapeValue, settings.separator, fields, formatsMap); @@ -46,7 +48,15 @@ export function createGenerateCsv(logger) { break; } - if (!builder.tryAppend(formatCsvValues(flattenHit(hit)) + '\n')) { + const flattened = flattenHit(hit); + const rows = formatCsvValues(flattened); + const rowsHaveFormulas = checkIfRowsHaveFormulas(flattened, fields); + + if (rowsHaveFormulas) { + csvContainsFormulas = true; + } + + if (!builder.tryAppend(rows + '\n')) { logger.warn('max Size Reached'); maxSizeReached = true; cancellationToken.cancel(); @@ -61,6 +71,7 @@ export function createGenerateCsv(logger) { return { content: builder.getString(), + csvContainsFormulas, maxSizeReached, size, }; diff --git a/x-pack/legacy/plugins/reporting/public/components/report_listing.tsx b/x-pack/legacy/plugins/reporting/public/components/report_listing.tsx index a1cf01be20232..5f542c397525e 100644 --- a/x-pack/legacy/plugins/reporting/public/components/report_listing.tsx +++ b/x-pack/legacy/plugins/reporting/public/components/report_listing.tsx @@ -46,6 +46,7 @@ interface Job { max_size_reached: boolean; attempts: number; max_attempts: number; + csv_contains_formulas: boolean; } interface Props { @@ -334,6 +335,21 @@ class ReportListingUi extends Component { /> ); + if (record.csv_contains_formulas) { + return ( + + {button} + + ); + } + if (record.max_size_reached) { return ( { max_size_reached: job._source.output ? job._source.output.max_size_reached : false, attempts: job._source.attempts, max_attempts: job._source.max_attempts, + csv_contains_formulas: job._source.output.csv_contains_formulas, }) ), }); diff --git a/x-pack/legacy/plugins/reporting/public/hacks/job_completion_notifier.js b/x-pack/legacy/plugins/reporting/public/hacks/job_completion_notifier.js index f90a045d376e6..c670a485d3ce5 100644 --- a/x-pack/legacy/plugins/reporting/public/hacks/job_completion_notifier.js +++ b/x-pack/legacy/plugins/reporting/public/hacks/job_completion_notifier.js @@ -65,8 +65,8 @@ uiModules.get('kibana') // the job completes, that way we don't give the user a toast to download their report if they can't. // NOTE: this should be looking at configuration rather than the existence of a navLink if (chrome.navLinks.has('kibana:management')) { - const managementUrl = chrome.navLinks.get('kibana:management').url; - const reportingSectionUrl = `${managementUrl}/kibana/reporting`; + const { baseUrl } = chrome.navLinks.get('kibana:management'); + const reportingSectionUrl = `${baseUrl}/kibana/reporting`; seeReportLink = (

+ ), + text: ( +

+

+ +

+ {seeReportLink} + {downloadReportButton} +
+ ), + 'data-test-subj': 'completeReportSuccess', + }); + } if (maxSizeReached) { return toastNotifications.addWarning({ diff --git a/x-pack/legacy/plugins/reporting/server/lib/esqueue/worker.js b/x-pack/legacy/plugins/reporting/server/lib/esqueue/worker.js index c71ec211804cd..25837e96650b0 100644 --- a/x-pack/legacy/plugins/reporting/server/lib/esqueue/worker.js +++ b/x-pack/legacy/plugins/reporting/server/lib/esqueue/worker.js @@ -186,6 +186,7 @@ export class Worker extends events.EventEmitter { docOutput.content_type = output.content_type || unknownMime; docOutput.max_size_reached = output.max_size_reached; docOutput.size = output.size; + docOutput.csv_contains_formulas = output.csv_contains_formulas; } else { docOutput.content = output || defaultOutput; docOutput.content_type = unknownMime; diff --git a/x-pack/legacy/plugins/reporting/server/routes/lib/get_document_payload.ts b/x-pack/legacy/plugins/reporting/server/routes/lib/get_document_payload.ts index f99f0fb0ce434..e9774fbe4a015 100644 --- a/x-pack/legacy/plugins/reporting/server/routes/lib/get_document_payload.ts +++ b/x-pack/legacy/plugins/reporting/server/routes/lib/get_document_payload.ts @@ -6,14 +6,34 @@ // @ts-ignore import contentDisposition from 'content-disposition'; +import * as _ from 'lodash'; // @ts-ignore import { oncePerServer } from '../../lib/once_per_server'; +import { CSV_JOB_TYPE } from '../../../common/constants'; + +interface ICustomHeaders { + [x: string]: any; +} const DEFAULT_TITLE = 'report'; const getTitle = (exportType: any, title?: string): string => `${title || DEFAULT_TITLE}.${exportType.jobContentExtension}`; +const getReportingHeaders = (output: any, exportType: any) => { + const metaDataHeaders: ICustomHeaders = {}; + + if (exportType.jobType === CSV_JOB_TYPE) { + const csvContainsFormulas = _.get(output, 'csv_contains_formulas', false); + const maxSizedReach = _.get(output, 'max_size_reached', false); + + metaDataHeaders['kbn-csv-contains-formulas'] = csvContainsFormulas; + metaDataHeaders['kbn-max-size-reached'] = maxSizedReach; + } + + return metaDataHeaders; +}; + function getDocumentPayloadFn(server: any) { const exportTypesRegistry = server.plugins.reporting.exportTypesRegistry; @@ -29,12 +49,14 @@ function getDocumentPayloadFn(server: any) { function getCompleted(output: any, jobType: string, title: any) { const exportType = exportTypesRegistry.get((item: any) => item.jobType === jobType); const filename = getTitle(exportType, title); + const headers = getReportingHeaders(output, exportType); return { statusCode: 200, content: encodeContent(output.content, exportType), contentType: output.content_type, headers: { + ...headers, 'Content-Disposition': contentDisposition(filename, { type: 'inline' }), }, }; diff --git a/x-pack/legacy/plugins/reporting/types.d.ts b/x-pack/legacy/plugins/reporting/types.d.ts index 235bf04e58437..62f83bc93d983 100644 --- a/x-pack/legacy/plugins/reporting/types.d.ts +++ b/x-pack/legacy/plugins/reporting/types.d.ts @@ -183,6 +183,7 @@ export interface JobDocOutputExecuted { content: string | null; // defaultOutput is null max_size_reached: boolean; size: number; + csv_contains_formulas?: string[]; } export interface ESQueueWorker { From 33f0fa0eb6cc07cac4bed0c7ebb09673e42f16f5 Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Tue, 25 Jun 2019 15:29:06 -0700 Subject: [PATCH 2/3] Config flag for checking if CSV's contain formulas --- .../csv/server/__tests__/execute_job.js | 24 +++++++++++++++++++ .../export_types/csv/server/execute_job.js | 1 + .../csv/server/lib/generate_csv.js | 2 +- x-pack/legacy/plugins/reporting/index.js | 1 + 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js b/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js index 8729f5fe30f64..fb82cb45ee62f 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/__tests__/execute_job.js @@ -298,6 +298,7 @@ describe('CSV Execute Job', function () { describe('Cells with formula values', async () => { it('returns `csv_contains_formulas` when cells contain formulas', async function () { + mockServer.config().get.withArgs('xpack.reporting.csv.checkForFormulas').returns(true); callWithRequestStub.onFirstCall().returns({ hits: { hits: [{ _source: { 'one': '=SUM(A1:A2)', 'two': 'bar' } }] @@ -318,6 +319,7 @@ describe('CSV Execute Job', function () { }); it('returns warnings when headings contain formulas', async function () { + mockServer.config().get.withArgs('xpack.reporting.csv.checkForFormulas').returns(true); callWithRequestStub.onFirstCall().returns({ hits: { hits: [{ _source: { '=SUM(A1:A2)': 'foo', 'two': 'bar' } }] @@ -338,6 +340,7 @@ describe('CSV Execute Job', function () { }); it('returns no warnings when cells have no formulas', async function () { + mockServer.config().get.withArgs('xpack.reporting.csv.checkForFormulas').returns(true); callWithRequestStub.onFirstCall().returns({ hits: { hits: [{ _source: { 'one': 'foo', 'two': 'bar' } }] @@ -356,6 +359,27 @@ describe('CSV Execute Job', function () { expect(csvContainsFormulas).to.equal(false); }); + + it('returns no warnings when configured not to', async () => { + mockServer.config().get.withArgs('xpack.reporting.csv.checkForFormulas').returns(false); + callWithRequestStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { 'one': '=SUM(A1:A2)', 'two': 'bar' } }] + }, + _scroll_id: 'scrollId' + }); + + const executeJob = executeJobFactory(mockServer); + const jobParams = { + headers: encryptedHeaders, + fields: [ 'one', 'two' ], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }; + const { csv_contains_formulas: csvContainsFormulas } = await executeJob(jobParams, cancellationToken); + + expect(csvContainsFormulas).to.equal(false); + }); }); describe('Elasticsearch call errors', function () { diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js index 5a130655780ec..564163275751c 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.js @@ -102,6 +102,7 @@ function executeJobFn(server) { formatsMap, settings: { ...uiSettings, + checkForFormulas: config.get('xpack.reporting.csv.checkForFormulas'), maxSizeBytes: config.get('xpack.reporting.csv.maxSizeBytes'), scroll: config.get('xpack.reporting.csv.scroll'), }, diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js index 8721637c3382b..7d69b26aedd8b 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.js @@ -50,7 +50,7 @@ export function createGenerateCsv(logger) { const flattened = flattenHit(hit); const rows = formatCsvValues(flattened); - const rowsHaveFormulas = checkIfRowsHaveFormulas(flattened, fields); + const rowsHaveFormulas = settings.checkForFormulas && checkIfRowsHaveFormulas(flattened, fields); if (rowsHaveFormulas) { csvContainsFormulas = true; diff --git a/x-pack/legacy/plugins/reporting/index.js b/x-pack/legacy/plugins/reporting/index.js index ea659993d15af..98a903be061ee 100644 --- a/x-pack/legacy/plugins/reporting/index.js +++ b/x-pack/legacy/plugins/reporting/index.js @@ -127,6 +127,7 @@ export const reporting = (kibana) => { }).default() }).default(), csv: Joi.object({ + checkForFormulas: Joi.boolean().default(true), enablePanelActionDownload: Joi.boolean().default(false), maxSizeBytes: Joi.number().integer().default(1024 * 1024 * 10), // bytes in a kB * kB in a mB * 10 scroll: Joi.object({ From ea1ec66da554167e27c6b722cb0be3df7769365e Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Tue, 25 Jun 2019 16:10:59 -0700 Subject: [PATCH 3/3] Fixing snapshots --- .../legacy/plugins/reporting/__snapshots__/index.test.js.snap | 4 ++++ 1 file changed, 4 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 ed5d0d236e603..3397f0ae39b08 100644 --- a/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap +++ b/x-pack/legacy/plugins/reporting/__snapshots__/index.test.js.snap @@ -25,6 +25,7 @@ Object { "zoom": 2, }, "csv": Object { + "checkForFormulas": true, "enablePanelActionDownload": false, "maxSizeBytes": 10485760, "scroll": Object { @@ -86,6 +87,7 @@ Object { "zoom": 2, }, "csv": Object { + "checkForFormulas": true, "enablePanelActionDownload": false, "maxSizeBytes": 10485760, "scroll": Object { @@ -146,6 +148,7 @@ Object { "zoom": 2, }, "csv": Object { + "checkForFormulas": true, "enablePanelActionDownload": false, "maxSizeBytes": 10485760, "scroll": Object { @@ -207,6 +210,7 @@ Object { "zoom": 2, }, "csv": Object { + "checkForFormulas": true, "enablePanelActionDownload": false, "maxSizeBytes": 10485760, "scroll": Object {