[Security Solution] Open alerts with an associated template in the template view #123333

kqualters-elastic · 2022-01-19T01:11:05Z

Summary

Resolves #123300 resolves #120898 and resolves #123370 alerts with an associated timeline template now use the derived notes, filters, data providers and query with the other defaults.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

madirey · 2022-01-19T02:59:38Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

-          const alertGroupId = alertGroupIdField?.length
-            ? alertGroupIdField[0]
-            : 'unknown-group-id';
+          const alertGroupId = alertGroupIdField?.length ? alertGroupIdField : 'unknown-group-id';


alertGroupIdField ?? 'unknown-group-id'?

+1 to @madirey suggestion.

michaelolo24

Thanks Kevin for getting these fixes in! Tested with an EQL Rule, Threshold Rule, Prepackaged Timeline Templates, and a custom template. All worked as expected! LGTM 🚀

…lert id

andrew-goldstein · 2022-01-19T19:40:21Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

-      dataProviders: [],
-      filters: buildAlertsKqlFilter('_id', alertsIds),
+      filters: [],
+      dataProviders: alertsIds.map((id) => {


👍 appreciate this, as it ensures the _id is populated in data providers vs filters (as before)

madirey

Thanks!!

YulNaumenko · 2022-01-20T01:19:49Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

    const queryMatchField = getFieldKey(ecs, ALERT_GROUP_ID);
-    const alertGroupId = alertGroupIdField?.length ? alertGroupIdField[0] : 'unknown-group-id';
+    const alertGroupId = alertGroupIdField?.length ? alertGroupIdField : 'unknown-group-id';


If alertGroupIdField is not array any more, why we have this length check alertGroupIdField?.length?

YulNaumenko · 2022-01-20T01:56:23Z

@kqualters-elastic thank you for the great work! Are there any chances to have some unit tests for x-pack/plugins/security_solution/public/helpers.tsx and x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx?

michaelolo24 · 2022-01-20T13:48:44Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

@@ -344,9 +359,7 @@ export const buildEqlDataProviderOrFilter = (
        'signal.group.id',


One more thing, can we update this to ALERT_GROUP_ID per the sync from yesterday? Thanks!

MadameSheema · 2022-01-24T09:55:02Z

@elasticmachine merge upstream

andrew-goldstein · 2022-01-25T00:34:02Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

-        if (!acc.includes(dateTimestamp.valueOf())) {
-          return [...acc, dateTimestamp.valueOf()];
-        }
+      const dateTimestamp = new Date(item.timestamp ?? '');


is it ok that new Date('') returns an Invalid Date?

andrew-goldstein · 2022-01-25T00:56:27Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

-          const alertGroupId = alertGroupIdField?.length
-            ? alertGroupIdField[0]
-            : 'unknown-group-id';
+          const alertGroupId = !Array.isArray(alertGroupIdField)


nit: consider removing the negation via:

const alertGroupId = Array.isArray(alertGroupIdField) ? alertGroupIdField[0] : alertGroupIdField;

michaelolo24 · 2022-01-25T01:35:38Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

@@ -137,12 +135,12 @@ export const determineToAndFrom = ({ ecs }: { ecs: Ecs[] | Ecs }) => {
  const ecsData = ecs as Ecs;
  const ruleFrom = getField(ecsData, ALERT_RULE_FROM);
  const elapsedTimeRule = moment.duration(
-    moment().diff(dateMath.parse(ruleFrom != null ? ruleFrom[0] : 'now-0s'))
+    moment().diff(dateMath.parse(ruleFrom != null ? ruleFrom[0] : 'now-1d'))


michaelolo24 · 2022-01-25T01:36:46Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

    return {
      dataProviders: [],
      filters: buildAlertsKqlFilter(
-        'signal.group.id',
+        ALERT_GROUP_ID,


andrew-goldstein · 2022-01-25T02:37:33Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

  const { to, from } = determineToAndFrom({ ecs });

  // For now we do not want to populate the template timeline if we have alertIds
-  if (!isEmpty(timelineId) && isEmpty(alertIds)) {
+  if (!isEmpty(timelineId) && isThresholdRule(ecsData) === false) {


to ensure the template is applied to threshold rules, consider the following change suggested by @michaelolo24 :

if (!isEmpty(timelineId) && !isEmpty(alertIds)) {

…a template, fix test that should never have passed

andrew-goldstein

Thanks for all the thought and effort you put into this fix @kqualters-elastic! 🙏
Desk tested locally
LGTM 🚀

michaelolo24

Discussed recent changes offline. Re-approving. Thanks for the changes @kqualters-elastic! LGTM

For posterity: retested the following rule & rule - template matches and everything worked as expected

Query Rule no templates attached Works as expected
Query Rule with custom template attached Works as expected
Query Rule with network template attached Works as expected
Query Rule with process template attached Works as expected
Threshold Rule no template attached Works as expected
Threshold Rule with process template attached Works as expected
EQL Rule with no template attached Works as expected
EQL Rule with process template attached Works as expected

Regarding the non-alert views:

Hosts Page (Events, External Alerts)
Network Page (External Alerts)

Tested and they properly load an event in the timeline and fill the query builder.

YulNaumenko

LGTM!

kibana-ci · 2022-01-25T06:23:29Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	4.6MB	4.6MB	+360.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	245.7KB	245.6KB	-62.0B

History

💚 Build #19264 succeeded 59ab4aa
💚 Build #19006 succeeded 30fa320
💚 Build #18515 succeeded 2c32e9f
💚 Build #18425 succeeded 0e616ba
💚 Build #18261 succeeded 1c6d772

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2022-01-25T06:39:23Z

The following labels were identified as gaps in your version labels and will be added automatically:

v8.1.0

If any of these should not be on your pull request, please manually remove them.

@timestamp

…mplate view (elastic#123333) * Open alerts with a template, with a template * Add default values back instead of template derived ones * Use data providers over filters always, set timeline description to alert id * Remove prepopulated description from non threshold alerts * Open any event in timeline, use correct timestamp * Remove unneeded @timestamp, make sure alertsEcsData is not empty array * Add basic getField tests * Explicity check if alertGroupId is an array instead of using length * Always use a valid date for time range * Only use filter if more than 1 alert is present * Possibly controversial change to calculate threshold time range with a template, fix test that should never have passed * Create threshold timeline in separate function * Use better type for createTimeline passed to createThresholdTimeline * Invert negation as suggested in pr comment * Use template timeline filters/query/data providers for threshold alerts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit cef886f)

kibanamachine · 2022-01-25T06:42:03Z

💚 All backports created successfully

Status	Branch	Result
✅	8.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@timestamp

…mplate view (#123333) (#123689) * Open alerts with a template, with a template * Add default values back instead of template derived ones * Use data providers over filters always, set timeline description to alert id * Remove prepopulated description from non threshold alerts * Open any event in timeline, use correct timestamp * Remove unneeded @timestamp, make sure alertsEcsData is not empty array * Add basic getField tests * Explicity check if alertGroupId is an array instead of using length * Always use a valid date for time range * Only use filter if more than 1 alert is present * Possibly controversial change to calculate threshold time range with a template, fix test that should never have passed * Create threshold timeline in separate function * Use better type for createTimeline passed to createThresholdTimeline * Invert negation as suggested in pr comment * Use template timeline filters/query/data providers for threshold alerts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit cef886f) Co-authored-by: Kevin Qualters <56408403+kqualters-elastic@users.noreply.github.com>

Open alerts with a template, with a template

1c6d772

kqualters-elastic added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed Team:Threat Hunting:Investigations Security Solution Investigations Team v8.0.1 labels Jan 19, 2022

kqualters-elastic requested a review from a team as a code owner January 19, 2022 01:11

kqualters-elastic marked this pull request as draft January 19, 2022 01:14

madirey reviewed Jan 19, 2022

View reviewed changes

michaelolo24 mentioned this pull request Jan 19, 2022

fix weird eql bugs #123315

Closed

9 tasks

Add default values back instead of template derived ones

0e616ba

kqualters-elastic marked this pull request as ready for review January 19, 2022 16:39

michaelolo24 approved these changes Jan 19, 2022

View reviewed changes

kqualters-elastic added 2 commits January 19, 2022 09:23

Use data providers over filters always, set timeline description to a…

5be2880

…lert id

Remove prepopulated description from non threshold alerts

2c32e9f

andrew-goldstein reviewed Jan 19, 2022

View reviewed changes

andrew-goldstein mentioned this pull request Jan 19, 2022

[Security Solution] Innacurate generic timeline template #123370

Closed

madirey approved these changes Jan 19, 2022

View reviewed changes

YulNaumenko reviewed Jan 20, 2022

View reviewed changes

michaelolo24 reviewed Jan 20, 2022

View reviewed changes

kibanamachine and others added 4 commits January 24, 2022 04:55

Merge branch 'main' into open-in-timeline-cleanup

30fa320

Open any event in timeline, use correct timestamp

597b4c9

Remove unneeded @timestamp, make sure alertsEcsData is not empty array

6f72615

Add basic getField tests

c8a86da

andrew-goldstein reviewed Jan 25, 2022

View reviewed changes

Explicity check if alertGroupId is an array instead of using length

f590124

andrew-goldstein reviewed Jan 25, 2022

View reviewed changes

Always use a valid date for time range

6fa41a9

Only use filter if more than 1 alert is present

59ab4aa

michaelolo24 reviewed Jan 25, 2022

View reviewed changes

andrew-goldstein reviewed Jan 25, 2022

View reviewed changes

kqualters-elastic added 5 commits January 24, 2022 22:34

Possibly controversial change to calculate threshold time range with …

6fd6265

…a template, fix test that should never have passed

Create threshold timeline in separate function

9b2b89e

Use better type for createTimeline passed to createThresholdTimeline

5e5ab70

Invert negation as suggested in pr comment

84dd174

Use template timeline filters/query/data providers for threshold alerts

12c42d3

andrew-goldstein approved these changes Jan 25, 2022

View reviewed changes

michaelolo24 approved these changes Jan 25, 2022

View reviewed changes

YulNaumenko approved these changes Jan 25, 2022

View reviewed changes

kqualters-elastic merged commit cef886f into elastic:main Jan 25, 2022

kqualters-elastic deleted the open-in-timeline-cleanup branch January 25, 2022 06:39

kibanamachine added the v8.1.0 label Jan 25, 2022

kibanamachine mentioned this pull request Jan 25, 2022

[8.0] [Security Solution] Open alerts with an associated template in the template view (#123333) #123689

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Open alerts with an associated template in the template view #123333

[Security Solution] Open alerts with an associated template in the template view #123333

kqualters-elastic commented Jan 19, 2022 •

edited

Loading

madirey Jan 19, 2022

YulNaumenko Jan 20, 2022

michaelolo24 left a comment •

edited

Loading

andrew-goldstein Jan 19, 2022

madirey left a comment

YulNaumenko Jan 20, 2022

YulNaumenko commented Jan 20, 2022

michaelolo24 Jan 20, 2022 •

edited

Loading

MadameSheema commented Jan 24, 2022

andrew-goldstein Jan 25, 2022

kqualters-elastic Jan 25, 2022

andrew-goldstein Jan 25, 2022

michaelolo24 Jan 25, 2022

michaelolo24 Jan 25, 2022

andrew-goldstein Jan 25, 2022

andrew-goldstein left a comment

michaelolo24 left a comment •

edited

Loading

YulNaumenko left a comment

kibana-ci commented Jan 25, 2022

kibanamachine commented Jan 25, 2022

kibanamachine commented Jan 25, 2022

		@@ -344,9 +359,7 @@ export const buildEqlDataProviderOrFilter = (
		'signal.group.id',

[Security Solution] Open alerts with an associated template in the template view #123333

[Security Solution] Open alerts with an associated template in the template view #123333

Conversation

kqualters-elastic commented Jan 19, 2022 • edited Loading

Summary

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madirey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YulNaumenko commented Jan 20, 2022

michaelolo24 Jan 20, 2022 • edited Loading

Choose a reason for hiding this comment

MadameSheema commented Jan 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew-goldstein left a comment

Choose a reason for hiding this comment

michaelolo24 left a comment • edited Loading

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

kibana-ci commented Jan 25, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

kibanamachine commented Jan 25, 2022

kibanamachine commented Jan 25, 2022

💚 All backports created successfully

Questions ?

kqualters-elastic commented Jan 19, 2022 •

edited

Loading

michaelolo24 left a comment •

edited

Loading

michaelolo24 Jan 20, 2022 •

edited

Loading

michaelolo24 left a comment •

edited

Loading