From 1cb213c04944168bc83804c8572cbfa648f9eda4 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 2 Apr 2021 16:12:53 -0500 Subject: [PATCH 1/7] First pass at rebuilding nested object structure from fields response * Always requests TIMELINE_CTI_FIELDS as part of request This only works for one level of nesting; will be extending tests to allow for multiple levels momentarily. --- .../timeline/factory/events/all/constants.ts | 10 ++ .../factory/events/all/helpers.test.ts | 122 +++++++++++++++++- .../timeline/factory/events/all/helpers.ts | 55 +++++--- .../timeline/factory/events/mocks.ts | 2 +- 4 files changed, 167 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/constants.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/constants.ts index 15d0e2d5494b8..29b0df9e4bbf7 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/constants.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/constants.ts @@ -5,6 +5,15 @@ * 2.0. */ +export const TIMELINE_CTI_FIELDS = [ + 'threat.indicator.event.dataset', + 'threat.indicator.event.reference', + 'threat.indicator.matched.atomic', + 'threat.indicator.matched.field', + 'threat.indicator.matched.type', + 'threat.indicator.provider', +]; + export const TIMELINE_EVENTS_FIELDS = [ '@timestamp', 'signal.status', @@ -230,4 +239,5 @@ export const TIMELINE_EVENTS_FIELDS = [ 'zeek.ssl.established', 'zeek.ssl.resumed', 'zeek.ssl.version', + ...TIMELINE_CTI_FIELDS, ]; diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts index 61af6a7664faa..54afaf764da21 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts @@ -7,7 +7,7 @@ import { EventHit } from '../../../../../../common/search_strategy'; import { TIMELINE_EVENTS_FIELDS } from './constants'; -import { formatTimelineData } from './helpers'; +import { buildObjectFromField, formatTimelineData } from './helpers'; import { eventHit } from '../mocks'; describe('#formatTimelineData', () => { @@ -42,12 +42,12 @@ describe('#formatTimelineData', () => { value: ['beats-ci-immutable-ubuntu-1804-1605624279743236239'], }, { - field: 'source.geo.location', - value: [`{"lon":118.7778,"lat":32.0617}`], + field: 'threat.indicator.matched.field', + value: ['matched_field', 'other_matched_field', 'matched_field_2'], }, { - field: 'threat.indicator.matched.field', - value: ['matched_field', 'matched_field_2'], + field: 'source.geo.location', + value: [`{"lon":118.7778,"lat":32.0617}`], }, ], ecs: { @@ -94,6 +94,34 @@ describe('#formatTimelineData', () => { user: { name: ['jenkins'], }, + threat: { + indicator: [ + { + event: { + dataset: [], + reference: [], + }, + matched: { + atomic: ['matched_atomic'], + field: ['matched_field', 'other_matched_field'], + type: [], + }, + provider: ['yourself'], + }, + { + event: { + dataset: [], + reference: [], + }, + matched: { + atomic: ['matched_atomic_2'], + field: ['matched_field_2'], + type: [], + }, + provider: ['other_you'], + }, + ], + }, }, }, }); @@ -371,4 +399,88 @@ describe('#formatTimelineData', () => { }, }); }); + + describe('buildObjectFromField', () => { + it('base case', () => { + expect(buildObjectFromField('@timestamp', eventHit)).toEqual({ + '@timestamp': ['2020-11-17T14:48:08.922Z'], + }); + }); + + it('another base case', () => { + const event = { + fields: { + foo: [{ bar: ['baz'] }], + }, + }; + // @ts-expect-error nestedEvent is minimal + expect(buildObjectFromField('foo.bar', event, 'foo')).toEqual({ + foo: [{ bar: ['baz'] }], + }); + }); + + it('simple nested', () => { + const nestedEvent = { + fields: { + 'foo.bar': [ + { + baz: ['host.name'], + }, + ], + }, + }; + // @ts-expect-error nestedEvent is minimal + expect(buildObjectFromField('foo.bar.baz', nestedEvent, 'foo.bar')).toEqual({ + foo: { + bar: [ + { + baz: ['host.name'], + }, + ], + }, + }); + }); + + it('nested field', () => { + expect( + buildObjectFromField('threat.indicator.matched.atomic', eventHit, 'threat.indicator') + ).toEqual({ + threat: { + indicator: [ + { + matched: { + atomic: ['matched_atomic'], + }, + }, + { + matched: { + atomic: ['matched_atomic_2'], + }, + }, + ], + }, + }); + }); + + it('nested field with multiple values', () => { + expect( + buildObjectFromField('threat.indicator.matched.field', eventHit, 'threat.indicator') + ).toEqual({ + threat: { + indicator: [ + { + matched: { + field: ['matched_field', 'other_matched_field'], + }, + }, + { + matched: { + field: ['matched_field_2'], + }, + }, + ], + }, + }); + }); + }); }); diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index e5bb8cb7e14b7..a5425ed30d4cc 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { set } from '@elastic/safer-lodash-set'; import { get, has, merge, uniq } from 'lodash/fp'; import { EventHit, @@ -99,6 +100,40 @@ const getValuesFromFields = async ( ); }; +export const buildObjectFromField = ( + fieldPath: string, + hit: EventHit, + nestedParentPath?: string +) => { + if (nestedParentPath) { + const childPath = fieldPath.replace(`${nestedParentPath}.`, ''); + return set( + {}, + nestedParentPath, + (get(nestedParentPath, hit.fields) ?? []).map((nestedFields) => { + const value = get(childPath, nestedFields); + return set({}, childPath, toStringArray(value)); + }) + ); + } else { + const value = has(fieldPath, hit._source) + ? get(fieldPath, hit._source) + : get(fieldPath, hit.fields); + return set({}, fieldPath, toStringArray(value)); + } +}; + +/** + * If a prefix of our full field path is present as a field, we know that our field is nested + */ +const getNestedParentPath = (fieldPath: string, fields: Record = {}) => { + const fieldParts = fieldPath.split('.'); + return Object.keys(fields).find( + (field) => + field !== fieldPath && field === fieldParts.slice(0, field.split('.').length).join('.') + ); +}; + const mergeTimelineFieldsWithHit = async ( fieldName: string, flattenedFields: T, @@ -106,15 +141,12 @@ const mergeTimelineFieldsWithHit = async ( dataFields: readonly string[], ecsFields: readonly string[] ) => { + const nestedParentPath = getNestedParentPath(fieldName, hit.fields); if (fieldName != null || dataFields.includes(fieldName)) { - const fieldNameAsArray = fieldName.split('.'); - const nestedParentFieldName = Object.keys(hit.fields ?? []).find((f) => { - return f === fieldNameAsArray.slice(0, f.split('.').length).join('.'); - }); if ( has(fieldName, hit._source) || has(fieldName, hit.fields) || - nestedParentFieldName != null || + nestedParentPath != null || specialFields.includes(fieldName) ) { const objectWithProperty = { @@ -123,22 +155,13 @@ const mergeTimelineFieldsWithHit = async ( data: dataFields.includes(fieldName) ? [ ...get('node.data', flattenedFields), - ...(await getValuesFromFields(fieldName, hit, nestedParentFieldName)), + ...(await getValuesFromFields(fieldName, hit, nestedParentPath)), ] : get('node.data', flattenedFields), ecs: ecsFields.includes(fieldName) ? { ...get('node.ecs', flattenedFields), - // @ts-expect-error - ...fieldName.split('.').reduceRight( - // @ts-expect-error - (obj, next) => ({ [next]: obj }), - toStringArray( - has(fieldName, hit._source) - ? get(fieldName, hit._source) - : hit.fields[fieldName] - ) - ), + ...buildObjectFromField(fieldName, hit, nestedParentPath), } : get('node.ecs', flattenedFields), }, diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts index 13b7fe7051246..f7ebef4cc6a3d 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts @@ -40,7 +40,7 @@ export const eventHit = { 'source.geo.location': [{ coordinates: [118.7778, 32.0617], type: 'Point' }], 'threat.indicator': [ { - 'matched.field': ['matched_field'], + 'matched.field': ['matched_field', 'other_matched_field'], first_seen: ['2021-02-22T17:29:25.195Z'], provider: ['yourself'], type: ['custom'], From cab30d47ee5b8c88f99af12ac2fc72b864138fc2 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 2 Apr 2021 20:08:44 -0500 Subject: [PATCH 2/7] Build objects from arbitrary levels of nesting This is a recursive implementation, but recursion depth is limited to the number of levels of nesting, with arguments reducing in size as we go (i.e. logarithmic) --- .../factory/events/all/helpers.test.ts | 117 +++++++++++++++--- .../timeline/factory/events/all/helpers.ts | 42 ++++--- 2 files changed, 129 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts index 54afaf764da21..8ad0f7f31b6f7 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts @@ -7,7 +7,7 @@ import { EventHit } from '../../../../../../common/search_strategy'; import { TIMELINE_EVENTS_FIELDS } from './constants'; -import { buildObjectFromField, formatTimelineData } from './helpers'; +import { buildObjectForFieldPath, formatTimelineData } from './helpers'; import { eventHit } from '../mocks'; describe('#formatTimelineData', () => { @@ -400,27 +400,42 @@ describe('#formatTimelineData', () => { }); }); - describe('buildObjectFromField', () => { - it('base case', () => { - expect(buildObjectFromField('@timestamp', eventHit)).toEqual({ + describe('buildObjectForFieldPath', () => { + it('builds an object from a single non-nested field', () => { + expect(buildObjectForFieldPath('@timestamp', eventHit)).toEqual({ '@timestamp': ['2020-11-17T14:48:08.922Z'], }); }); - it('another base case', () => { - const event = { + it('does not misinterpret non-nested fields with a common prefix', () => { + // @ts-expect-error hit is minimal + const hit: EventHit = { + fields: { + 'foo.bar': ['baz'], + 'foo.barBaz': ['foo'], + }, + }; + + expect(buildObjectForFieldPath('foo.barBaz', hit)).toEqual({ + foo: { barBaz: ['foo'] }, + }); + }); + + it('builds an array of objects from a nested field', () => { + // @ts-expect-error hit is minimal + const hit: EventHit = { fields: { foo: [{ bar: ['baz'] }], }, }; - // @ts-expect-error nestedEvent is minimal - expect(buildObjectFromField('foo.bar', event, 'foo')).toEqual({ + expect(buildObjectForFieldPath('foo.bar', hit, 'foo')).toEqual({ foo: [{ bar: ['baz'] }], }); }); - it('simple nested', () => { - const nestedEvent = { + it('builds intermediate objects for nested fields', () => { + // @ts-expect-error nestedHit is minimal + const nestedHit: EventHit = { fields: { 'foo.bar': [ { @@ -429,8 +444,7 @@ describe('#formatTimelineData', () => { ], }, }; - // @ts-expect-error nestedEvent is minimal - expect(buildObjectFromField('foo.bar.baz', nestedEvent, 'foo.bar')).toEqual({ + expect(buildObjectForFieldPath('foo.bar.baz', nestedHit, 'foo.bar')).toEqual({ foo: { bar: [ { @@ -441,9 +455,9 @@ describe('#formatTimelineData', () => { }); }); - it('nested field', () => { + it('builds intermediate objects at multiple levels', () => { expect( - buildObjectFromField('threat.indicator.matched.atomic', eventHit, 'threat.indicator') + buildObjectForFieldPath('threat.indicator.matched.atomic', eventHit, 'threat.indicator') ).toEqual({ threat: { indicator: [ @@ -462,9 +476,9 @@ describe('#formatTimelineData', () => { }); }); - it('nested field with multiple values', () => { + it('preserves multiple values for a single leaf', () => { expect( - buildObjectFromField('threat.indicator.matched.field', eventHit, 'threat.indicator') + buildObjectForFieldPath('threat.indicator.matched.field', eventHit, 'threat.indicator') ).toEqual({ threat: { indicator: [ @@ -482,5 +496,76 @@ describe('#formatTimelineData', () => { }, }); }); + + describe('multiple levels of nested fields', () => { + let nestedHit: EventHit; + + beforeEach(() => { + // @ts-expect-error nestedHit is minimal + nestedHit = { + fields: { + 'nested_1.foo': [ + { + 'nested_2.bar': [ + { leaf: ['leaf_value'], leaf_2: ['leaf_2_value'] }, + { leaf_2: ['leaf_2_value_2', 'leaf_2_value_3'] }, + ], + }, + { + 'nested_2.bar': [ + { leaf: ['leaf_value_2'], leaf_2: ['leaf_2_value_4'] }, + { leaf: ['leaf_value_3'], leaf_2: ['leaf_2_value_5'] }, + ], + }, + ], + }, + }; + }); + + it('includes objects without the field', () => { + expect( + buildObjectForFieldPath('nested_1.foo.nested_2.bar.leaf', nestedHit, 'nested_1.foo') + ).toEqual({ + nested_1: { + foo: [ + { + nested_2: { + bar: [{ leaf: ['leaf_value'] }, { leaf: [] }], + }, + }, + { + nested_2: { + bar: [{ leaf: ['leaf_value_2'] }, { leaf: ['leaf_value_3'] }], + }, + }, + ], + }, + }); + }); + + it('groups multiple leaf values', () => { + expect( + buildObjectForFieldPath('nested_1.foo.nested_2.bar.leaf_2', nestedHit, 'nested_1.foo') + ).toEqual({ + nested_1: { + foo: [ + { + nested_2: { + bar: [ + { leaf_2: ['leaf_2_value'] }, + { leaf_2: ['leaf_2_value_2', 'leaf_2_value_3'] }, + ], + }, + }, + { + nested_2: { + bar: [{ leaf_2: ['leaf_2_value_4'] }, { leaf_2: ['leaf_2_value_5'] }], + }, + }, + ], + }, + }); + }); + }); }); }); diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index a5425ed30d4cc..8307d5692a9da 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -6,7 +6,8 @@ */ import { set } from '@elastic/safer-lodash-set'; -import { get, has, merge, uniq } from 'lodash/fp'; +import { get, has, isObject, merge, uniq } from 'lodash/fp'; +import { Ecs } from '../../../../../../common/ecs'; import { EventHit, TimelineEdges, @@ -100,21 +101,31 @@ const getValuesFromFields = async ( ); }; -export const buildObjectFromField = ( +type Fields = Record; + +const buildNestedObject = (fieldPath: string, fields: Fields): Partial => { + const nestedParentPath = getNestedParentPath(fieldPath, fields); + if (!nestedParentPath) { + return set({}, fieldPath, toStringArray(get(fieldPath, fields))); + } + + const subPath = fieldPath.replace(`${nestedParentPath}.`, ''); + const subFields = (get(nestedParentPath, fields) ?? []) as Fields[]; + + return set( + {}, + nestedParentPath, + subFields.map((subField) => buildNestedObject(subPath, subField)) + ); +}; + +export const buildObjectForFieldPath = ( fieldPath: string, hit: EventHit, nestedParentPath?: string -) => { +): Partial => { if (nestedParentPath) { - const childPath = fieldPath.replace(`${nestedParentPath}.`, ''); - return set( - {}, - nestedParentPath, - (get(nestedParentPath, hit.fields) ?? []).map((nestedFields) => { - const value = get(childPath, nestedFields); - return set({}, childPath, toStringArray(value)); - }) - ); + return buildNestedObject(fieldPath, hit.fields); } else { const value = has(fieldPath, hit._source) ? get(fieldPath, hit._source) @@ -126,7 +137,10 @@ export const buildObjectFromField = ( /** * If a prefix of our full field path is present as a field, we know that our field is nested */ -const getNestedParentPath = (fieldPath: string, fields: Record = {}) => { +const getNestedParentPath = (fieldPath: string, fields: Fields): string | undefined => { + if (!isObject(fields)) { + return; + } const fieldParts = fieldPath.split('.'); return Object.keys(fields).find( (field) => @@ -161,7 +175,7 @@ const mergeTimelineFieldsWithHit = async ( ecs: ecsFields.includes(fieldName) ? { ...get('node.ecs', flattenedFields), - ...buildObjectFromField(fieldName, hit, nestedParentPath), + ...buildObjectForFieldPath(fieldName, hit, nestedParentPath), } : get('node.ecs', flattenedFields), }, From 69fd5a6e9580f6dccd488f16904dfba29d80dace Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 2 Apr 2021 21:06:15 -0500 Subject: [PATCH 3/7] Simplify parsing logic, perf improvements * Order short-circuiting conditions by cost, ascending * Simplify object building for non-nested objects from fields * The non-nested case is the same as the base recursive case, so always call our recursive function if building from .fields * Simplify getNestedParentPath * We can do a few simple string comparison rather than building up multiple strings/arrays * Don't call getNestedParentPath unnecessarily, only if we have a field --- .../factory/events/all/helpers.test.ts | 20 +++------ .../timeline/factory/events/all/helpers.ts | 41 ++++++------------- 2 files changed, 19 insertions(+), 42 deletions(-) diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts index 8ad0f7f31b6f7..fa360d5945334 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts @@ -428,7 +428,7 @@ describe('#formatTimelineData', () => { foo: [{ bar: ['baz'] }], }, }; - expect(buildObjectForFieldPath('foo.bar', hit, 'foo')).toEqual({ + expect(buildObjectForFieldPath('foo.bar', hit)).toEqual({ foo: [{ bar: ['baz'] }], }); }); @@ -444,7 +444,7 @@ describe('#formatTimelineData', () => { ], }, }; - expect(buildObjectForFieldPath('foo.bar.baz', nestedHit, 'foo.bar')).toEqual({ + expect(buildObjectForFieldPath('foo.bar.baz', nestedHit)).toEqual({ foo: { bar: [ { @@ -456,9 +456,7 @@ describe('#formatTimelineData', () => { }); it('builds intermediate objects at multiple levels', () => { - expect( - buildObjectForFieldPath('threat.indicator.matched.atomic', eventHit, 'threat.indicator') - ).toEqual({ + expect(buildObjectForFieldPath('threat.indicator.matched.atomic', eventHit)).toEqual({ threat: { indicator: [ { @@ -477,9 +475,7 @@ describe('#formatTimelineData', () => { }); it('preserves multiple values for a single leaf', () => { - expect( - buildObjectForFieldPath('threat.indicator.matched.field', eventHit, 'threat.indicator') - ).toEqual({ + expect(buildObjectForFieldPath('threat.indicator.matched.field', eventHit)).toEqual({ threat: { indicator: [ { @@ -523,9 +519,7 @@ describe('#formatTimelineData', () => { }); it('includes objects without the field', () => { - expect( - buildObjectForFieldPath('nested_1.foo.nested_2.bar.leaf', nestedHit, 'nested_1.foo') - ).toEqual({ + expect(buildObjectForFieldPath('nested_1.foo.nested_2.bar.leaf', nestedHit)).toEqual({ nested_1: { foo: [ { @@ -544,9 +538,7 @@ describe('#formatTimelineData', () => { }); it('groups multiple leaf values', () => { - expect( - buildObjectForFieldPath('nested_1.foo.nested_2.bar.leaf_2', nestedHit, 'nested_1.foo') - ).toEqual({ + expect(buildObjectForFieldPath('nested_1.foo.nested_2.bar.leaf_2', nestedHit)).toEqual({ nested_1: { foo: [ { diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index 8307d5692a9da..970168108237a 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -6,7 +6,7 @@ */ import { set } from '@elastic/safer-lodash-set'; -import { get, has, isObject, merge, uniq } from 'lodash/fp'; +import { get, has, merge, uniq } from 'lodash/fp'; import { Ecs } from '../../../../../../common/ecs'; import { EventHit, @@ -103,7 +103,7 @@ const getValuesFromFields = async ( type Fields = Record; -const buildNestedObject = (fieldPath: string, fields: Fields): Partial => { +const buildObjectRecursive = (fieldPath: string, fields: Fields): Partial => { const nestedParentPath = getNestedParentPath(fieldPath, fields); if (!nestedParentPath) { return set({}, fieldPath, toStringArray(get(fieldPath, fields))); @@ -111,42 +111,27 @@ const buildNestedObject = (fieldPath: string, fields: Fields): Partial => { const subPath = fieldPath.replace(`${nestedParentPath}.`, ''); const subFields = (get(nestedParentPath, fields) ?? []) as Fields[]; - return set( {}, nestedParentPath, - subFields.map((subField) => buildNestedObject(subPath, subField)) + subFields.map((subField) => buildObjectRecursive(subPath, subField)) ); }; -export const buildObjectForFieldPath = ( - fieldPath: string, - hit: EventHit, - nestedParentPath?: string -): Partial => { - if (nestedParentPath) { - return buildNestedObject(fieldPath, hit.fields); - } else { - const value = has(fieldPath, hit._source) - ? get(fieldPath, hit._source) - : get(fieldPath, hit.fields); +export const buildObjectForFieldPath = (fieldPath: string, hit: EventHit): Partial => { + if (has(fieldPath, hit._source)) { + const value = get(fieldPath, hit._source); return set({}, fieldPath, toStringArray(value)); } + + return buildObjectRecursive(fieldPath, hit.fields); }; /** * If a prefix of our full field path is present as a field, we know that our field is nested */ -const getNestedParentPath = (fieldPath: string, fields: Fields): string | undefined => { - if (!isObject(fields)) { - return; - } - const fieldParts = fieldPath.split('.'); - return Object.keys(fields).find( - (field) => - field !== fieldPath && field === fieldParts.slice(0, field.split('.').length).join('.') - ); -}; +const getNestedParentPath = (fieldPath: string, fields: Fields): string | undefined => + Object.keys(fields).find((field) => field !== fieldPath && fieldPath.startsWith(`${field}.`)); const mergeTimelineFieldsWithHit = async ( fieldName: string, @@ -155,12 +140,12 @@ const mergeTimelineFieldsWithHit = async ( dataFields: readonly string[], ecsFields: readonly string[] ) => { - const nestedParentPath = getNestedParentPath(fieldName, hit.fields); if (fieldName != null || dataFields.includes(fieldName)) { + const nestedParentPath = getNestedParentPath(fieldName, hit.fields); if ( + nestedParentPath != null || has(fieldName, hit._source) || has(fieldName, hit.fields) || - nestedParentPath != null || specialFields.includes(fieldName) ) { const objectWithProperty = { @@ -175,7 +160,7 @@ const mergeTimelineFieldsWithHit = async ( ecs: ecsFields.includes(fieldName) ? { ...get('node.ecs', flattenedFields), - ...buildObjectForFieldPath(fieldName, hit, nestedParentPath), + ...buildObjectForFieldPath(fieldName, hit), } : get('node.ecs', flattenedFields), }, From 9f8bdad23e0580657c96a141b0cf5aa687a0ab68 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 2 Apr 2021 21:13:45 -0500 Subject: [PATCH 4/7] Simplify if branching By definition, nestedParentFieldName can never be equal to fieldName, which means there are only two branches here. --- .../timeline/factory/events/all/helpers.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index 970168108237a..6451702299793 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -77,18 +77,13 @@ const getValuesFromFields = async ( [fieldName]: get(fieldName, hit._source), }; } else { - if (nestedParentFieldName == null || nestedParentFieldName === fieldName) { + if (nestedParentFieldName == null) { fieldToEval = { [fieldName]: hit.fields[fieldName], }; - } else if (nestedParentFieldName != null) { - fieldToEval = { - [nestedParentFieldName]: hit.fields[nestedParentFieldName], - }; } else { - // fallback, should never hit fieldToEval = { - [fieldName]: [], + [nestedParentFieldName]: hit.fields[nestedParentFieldName], }; } } From 168612bcd441ea2c2bb5d8ba3ea7a46a332fc156 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 2 Apr 2021 21:32:25 -0500 Subject: [PATCH 5/7] Declare/export a more accurate fields type Each top-level field value can be either an array of leaf values (unknown[]), or an array of nested fields. --- .../security_solution/matrix_histogram/events/index.ts | 4 +++- .../search_strategy/timeline/factory/events/all/helpers.ts | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/common/search_strategy/security_solution/matrix_histogram/events/index.ts b/x-pack/plugins/security_solution/common/search_strategy/security_solution/matrix_histogram/events/index.ts index 53cdc7239f69d..b2e0461b0b9b8 100644 --- a/x-pack/plugins/security_solution/common/search_strategy/security_solution/matrix_histogram/events/index.ts +++ b/x-pack/plugins/security_solution/common/search_strategy/security_solution/matrix_histogram/events/index.ts @@ -26,10 +26,12 @@ export interface EventsActionGroupData { doc_count: number; } +export type Fields = Record; + export interface EventHit extends SearchHit { sort: string[]; _source: EventSource; - fields: Record; + fields: Fields; aggregations: { // eslint-disable-next-line @typescript-eslint/no-explicit-any [agg: string]: any; diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index 6451702299793..0b7310ace87ad 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -10,6 +10,7 @@ import { get, has, merge, uniq } from 'lodash/fp'; import { Ecs } from '../../../../../../common/ecs'; import { EventHit, + Fields, TimelineEdges, TimelineNonEcsData, } from '../../../../../../common/search_strategy'; @@ -96,8 +97,6 @@ const getValuesFromFields = async ( ); }; -type Fields = Record; - const buildObjectRecursive = (fieldPath: string, fields: Fields): Partial => { const nestedParentPath = getNestedParentPath(fieldPath, fields); if (!nestedParentPath) { From 8143acdf440d41d0bdf6a97deedad19534077169 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 2 Apr 2021 21:37:21 -0500 Subject: [PATCH 6/7] Remove unnecessary condition If fieldName is null or undefined, there is no reason to search for it in dataFields. Looking through the git history this looks to be dead code as a result of refactoring, as opposed to a legitimate bugfix, so I'm removing it. --- .../search_strategy/timeline/factory/events/all/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index 0b7310ace87ad..e569ba64a6f14 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -134,7 +134,7 @@ const mergeTimelineFieldsWithHit = async ( dataFields: readonly string[], ecsFields: readonly string[] ) => { - if (fieldName != null || dataFields.includes(fieldName)) { + if (fieldName != null) { const nestedParentPath = getNestedParentPath(fieldName, hit.fields); if ( nestedParentPath != null || From e5aada234d1f8961597a4ee4269ea47f4eea087e Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 7 Apr 2021 22:18:25 -0500 Subject: [PATCH 7/7] Fix failing tests * one was a test failure due to my modifying mock data * one may have been a legitimate bug where we don't handle a hit without a fields response; I need to follow up with Xavier to verify. --- .../timeline/factory/events/all/helpers.test.ts | 8 ++++++++ .../timeline/factory/events/all/helpers.ts | 3 ++- .../search_strategy/timeline/factory/events/mocks.ts | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts index fa360d5945334..030df02efa5f0 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts @@ -407,6 +407,14 @@ describe('#formatTimelineData', () => { }); }); + it('builds an object with no fields response', () => { + const { fields, ...fieldLessHit } = eventHit; + // @ts-expect-error fieldLessHit is intentionally missing fields + expect(buildObjectForFieldPath('@timestamp', fieldLessHit)).toEqual({ + '@timestamp': [], + }); + }); + it('does not misinterpret non-nested fields with a common prefix', () => { // @ts-expect-error hit is minimal const hit: EventHit = { diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts index e569ba64a6f14..8399b3331ee4b 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.ts @@ -124,7 +124,8 @@ export const buildObjectForFieldPath = (fieldPath: string, hit: EventHit): Parti /** * If a prefix of our full field path is present as a field, we know that our field is nested */ -const getNestedParentPath = (fieldPath: string, fields: Fields): string | undefined => +const getNestedParentPath = (fieldPath: string, fields: Fields | undefined): string | undefined => + fields && Object.keys(fields).find((field) => field !== fieldPath && fieldPath.startsWith(`${field}.`)); const mergeTimelineFieldsWithHit = async ( diff --git a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts index f7ebef4cc6a3d..7dc257ebb3fef 100644 --- a/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts +++ b/x-pack/plugins/security_solution/server/search_strategy/timeline/factory/events/mocks.ts @@ -259,8 +259,8 @@ export const eventDetailsFormattedFields = [ { category: 'threat', field: 'threat.indicator.matched.field', - values: ['matched_field', 'matched_field_2'], - originalValue: ['matched_field', 'matched_field_2'], + values: ['matched_field', 'other_matched_field', 'matched_field_2'], + originalValue: ['matched_field', 'other_matched_field', 'matched_field_2'], isObjectArray: false, }, {