Skip to content
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][Detections] Create Threshold-based Rule type #71371

Merged

Conversation

patrykkopycinski
Copy link
Contributor

@patrykkopycinski patrykkopycinski commented Jul 10, 2020

Summary

Implements #68409

  1. New rule type with the new Threshold field
    Zrzut ekranu 2020-07-13 o 10 56 03
  2. Threshold field suggestions list supports defined index patterns (if not specified we will check overall number of results)
    Zrzut ekranu 2020-07-13 o 10 56 17
  3. Threshold field description displays proper content for All results as well if they were aggregated
    Zrzut ekranu 2020-07-13 o 10 56 56
  4. Threshold rule-based signals get populated fields from the query and aggregation result(if defined)
    image
  5. Investigation in the timeline for threshold rule-based signals prepopulates to the timeline rule's query and aggregation field (if defined)
    image

Checklist

…-rule-type

# Conflicts:
#	x-pack/plugins/security_solution/public/graphql/introspection.json
#	x-pack/plugins/security_solution/public/graphql/types.ts
#	x-pack/plugins/security_solution/public/timelines/containers/index.gql_query.ts
#	x-pack/plugins/security_solution/server/graphql/ecs/schema.gql.ts
#	x-pack/plugins/security_solution/server/graphql/types.ts
#	x-pack/plugins/security_solution/server/lib/ecs_fields/index.ts
…-rule-type

# Conflicts:
#	x-pack/plugins/security_solution/public/detections/components/rules/threshold_input/index.tsx
@spong
Copy link
Member

spong commented Jul 13, 2020

I think we'll want to add the signal.rule.threshold fields to the signals_mapping.json so they're searchable.

if (rule.type === 'threshold') {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optionally you can use the io-ts type to verify the value is in bound -- either:
threshold.is(rule.threshold) (as imported from detection_engine/schemas/common/schemas.ts)
-or-
PositiveIntegerGreaterThanZero.is(rule.threshold.value)

@@ -4,8 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

/* eslint-disable complexity */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooweee! Did we finally make it? Haha 😅

I removed this locally and wasn't running into the error, so perhaps we're still good? Either way, I think the Alert Actions code has grown enough now that we can probably restructure and break it apart a bit further. This area of the codebase is only going to grow more and more now that we can programmatically declare actions on a per alert basis... 🙂

I've added this to my list of tech debt items for 7.10+, so nothing to worry about now.

Comment on lines +1073 to +1074
threshold?: Maybe<ToAny>;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the source declaration that results in these generated types for timeline? Looks like I need to add the new rule fields from #70288. This ensures these fields show up in the field browser for timeline, correct?

@@ -201,6 +197,18 @@ export const EventColumnView = React.memo<Props>(
: grouped.icon;
}, [button, closePopover, id, onClickCb, data, ecsData, timelineActions, isPopoverOpen]);

const handlePinClicked = useCallback(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ Thanks for saving the renders! 🚀

@@ -210,6 +210,8 @@ export const timelineQuery = gql`
to
filters
note
type
threshold
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, here it is (in reference to https://github.com/elastic/kibana/pull/71371/files#r453934881) -- question still though, what features are gained by adding the additional fields to the timeline gql, is it just the FieldBrowser/Event Details, or more?

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out, tested locally and reviewed code. Couple nits/optionals, and we'll definitely want to add the threshold_fields to the singals_mapping.json, but other than that LAFTM (Looks Absolutely Fantastic To Me!).

This is going to be such a great feature for our users -- in testing it was so awesome to be able to analyze a bundle of failed auth events by host within Timeline. Great work here @patrykkopycinski, really appreciate it! 🙂 🚀 🎉

@patrykkopycinski
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
securitySolution 761 +3 758

History

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:enhancement Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants