-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON #92025
[Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON #92025
Conversation
field: 'agent.id', | ||
originalValue: '5de03d5f-52f3-482e-91d4-853c7de073c3', | ||
values: ['5de03d5f-52f3-482e-91d4-853c7de073c3'], | ||
field: 'cloud.project.id', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I "de-alphabetized" this mock data to ensure that our function puts it in alphabetical order
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
@@ -42,6 +42,18 @@ describe('#getDataFromFieldsHits', () => { | |||
'process.executable': [ | |||
'/var/lib/jenkins/workspace/Beats_beats_PR-22624/.gvm/versions/go1.14.7.linux.amd64/bin/go', | |||
], | |||
'threat.indicator': [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding this data (result on 227) tests toObjectArrayOfStrings
x-pack/plugins/security_solution/public/common/components/event_details/json_view.tsx
Outdated
Show resolved
Hide resolved
<FormattedFieldValue | ||
key={`plain-column-renderer-formatted-field-value-${timelineId}-${columnName}-${eventId}-${field.id}-${value}`} | ||
key={`plain-column-renderer-formatted-field-value-${timelineId}-${columnName}-${eventId}-${field.id}-${value}-${i}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, were the ${eventId}-${field.id}-${value}
not unique? I ask because we ran into an issue on the resolver when we introduced indices into keys because reacts resolution on re-renders would be jumbled if the data changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if this was before or after i filtered for unique values though, ill take a second look
}, | ||
[] | ||
); | ||
edges = await parseSequences(response.rawResponse.body.hits.sequences, options.fieldRequested); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏾 nice cleanup
@@ -290,7 +267,7 @@ describe('#formatTimelineData', () => { | |||
}; | |||
|
|||
expect( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the await goes outside the expect? https://jestjs.io/docs/en/tutorial-async#asyncawait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think thats for with .resolves
In the first example:
it('works with async/await', async () => {
expect.assertions(1);
const data = await user.getUserName(4);
expect(data).toEqual('Mark');
});
so mine is like this, but instead of using a variable data
i put the await function call directly in the ()
because it will run what is inside the ()
before evaluating the expect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephmilovic one other thing I noticed: sorting on those nested fields causes the query to fail: I don't think that should block this effort, but I wanted to make sure we're aware of it moving forward. The semantics of sorting on a nested field are unclear in the existing UI; would it be possible to disable sorting on those fields for now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and it looks good! I was able to load the sample data and saw the nested objects properly formatted. Only had a few code comments, but nothing breaking. Feel free to address in a separate PR. Thanks for doing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pulled and tested locally, does fix the issues. might make sense to add tests for isObjectArray otherwise lgtm pending @michaelolo24 's comment about await
good catch should be able to disable sorting |
@@ -86,6 +86,7 @@ export const HeaderComponent: React.FC<Props> = ({ | |||
getManageTimelineById, | |||
timelineId, | |||
]); | |||
const showSortingCapability = !isEqlOn && !(header.subType && header.subType.nested); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disable sorting on nested fields, not working.
The JSON fields are now sorted, which unfortunately caused this test to fail.
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
* master: (42 commits) [Lens] Introduces new chart switcher (elastic#91844) [Lens] fix selection when dragging (elastic#93034) Converts usage collection README to .mdx (elastic#92982) Fix expanding document when using saved search data grid (elastic#92999) [SECURITY SOLUTIONS] Bug case connector (elastic#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942) [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139) [DOCS] Fixes links for machine learning alerts (elastic#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667) Bump ems landing page to 7.12 (elastic#93065) [App Search] Implement various Relevance Tuning states and form actions (elastic#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092) Hide instances latency distribution chart (elastic#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087) [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (elastic#92932) ...
… playwright-ftr-e2e * 'playwright-ftr-e2e' of github.com:shahzad31/kibana: (38 commits) [chore] Enable core's eslint rule: `@ts-expect-error` (elastic#93086) [Lens] Introduces new chart switcher (elastic#91844) [Lens] fix selection when dragging (elastic#93034) Converts usage collection README to .mdx (elastic#92982) Fix expanding document when using saved search data grid (elastic#92999) [SECURITY SOLUTIONS] Bug case connector (elastic#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942) [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139) [DOCS] Fixes links for machine learning alerts (elastic#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667) Bump ems landing page to 7.12 (elastic#93065) [App Search] Implement various Relevance Tuning states and form actions (elastic#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092) Hide instances latency distribution chart (elastic#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087) ...
… ilm/rollup-v2-action * 'ilm/rollup-v2-action' of github.com:elastic/kibana: (30 commits) Fix expanding document when using saved search data grid (#92999) [SECURITY SOLUTIONS] Bug case connector (#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (#92942) [APM] Fix hidden search bar in error pages while loading (#84476) (#93139) [DOCS] Fixes links for machine learning alerts (#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (#92667) Bump ems landing page to 7.12 (#93065) [App Search] Implement various Relevance Tuning states and form actions (#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (#93092) Hide instances latency distribution chart (#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (#93087) [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (#92932) [Security Solution][Detecttions] Indicator enrichment tweaks (#92989) [Maps] fix fit to data on heatmap not working (#92697) [Security Solution][Endpoint][Admin] Fixes policy sticky footer save test (#92919) ...
Summary
Resolves the following issues in timeline related to the ES Fields API change:
#91424
#90355
#90222
#91426
#90808
Does NOT resolve: #89784 Dragging a nested field to timeline does not generate the correct query
+
button works, just dragging creates bad query+
Filter also needs display to be fixed, but is querying correctlyClick here to see the nested filter issue
Updates the timeline event details query dsl to include
fields: ['*']
and_source: true
in order to ensure we are getting all fields, even unmapped. Array values will be stringifiedAlso alphabetizes the JSON view to make it nice and easy to read for Prince @spong 👑
I also fixed a bug I noticed that made stringified object arrays into draggables, yikes:
I changed this to render as an undraggable string:
FWIW, the actual values were already being parsed correctly into draggable below:
To test:
post the below data to your siem-signals-default index. on the detections table page, search for
signal.rule.name": "GRRRRRowl"
, Ensure the threat.indicator and source.geo.location appear appropriately as draggables and strings where the full object is listed.