-
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] Add Alert Suppression editable component #198673
[Security Solution] Add Alert Suppression editable component #198673
Conversation
1a42a9c
to
ad5db96
Compare
fieldName: UpgradeableEsqlFields; | ||
} | ||
|
||
export function EsQlRuleFieldEdit({ fieldName }: EqQlRuleFieldEditProps) { |
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.
Shall we make it Esql
with a lowercase "q" for consistency? I see that in other parts of the codebase it's written as Esql
.
export function AlertSuppressionEditAdapter({ | ||
finalDiffableRule, | ||
}: RuleFieldEditComponentProps): JSX.Element { | ||
const { indexPattern: dataView } = useRuleIndexPattern({ |
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's a getRuleIndexPatternParameters
function in kql_query_edit.tsx. We might reuse it to simplify this conditional logic.
const defaultIndexPattern = useDefaultIndexPattern();
const indexPatternParameters = getRuleIndexPatternParameters(
finalDiffableRule,
defaultIndexPattern
);
const { indexPattern: dataView } = useRuleIndexPattern(indexPatternParameters);
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 finally decided to refactor the implementation to get rid of useRuleIndexPattern()
.
? GroupByOptions.PerTimePeriod | ||
: GroupByOptions.PerRuleExecution, | ||
[SUPPRESSION_DURATION]: alertSuppression?.duration ?? { | ||
value: 5, |
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.
Do you think it would add value if we extract this default object into a constant and then reuse it here, on the rule editing page?
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.
Yes, finally extracted it into a constant.
disabledText, | ||
}: SuppressionFieldsSelectorProps): JSX.Element { | ||
return ( | ||
<> |
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.
Can fragment wrappers (<>
) here and around UseField
be removed? Do they serve any purpose?
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.
It was a refactoring artefact.
8806cfc
to
5eb6aa4
Compare
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
b48f32b
to
b9ae744
Compare
9f4d426
to
68f8fe6
Compare
@vitaliidm The behavior you discovered in #198673 (comment) is technically correct. User doesn't select an existing value so empty alert suppression fields array is submitted leading to removing alert suppression in the upgrade. Additionally this behavor matches with existing on rule creation/editing pages. Though I agree it doesn't look like the best UX. And we should consider improving it before the Prebuilt Rules Customization feature release. It doesn't look like a critical issue and shouldn't block this PR. WDYT? UPD: I created a ticket to collect improvement tasks. |
I have discussed this with the team and consensus was we should not introduce any potential issue in this PR and should handle it separately. cc: @yctercero |
30ae31d
to
162b3a9
Compare
@vitaliidm I rolled back terms aggregation fields for threshold rules in 162b3a9. |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
cc @maximpn |
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.
desk tested and code LGTM for the Threat Hunting Investigations team
Starting backport for target branches: 8.x |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…198673) (#199809) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Add Alert Suppression editable component (#198673)](#198673) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maxim Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-12T14:46:39Z","message":"[Security Solution] Add Alert Suppression editable component (#198673)\n\n**Partially addresses:** https://github.com/elastic/kibana/issues/171520\r\n\r\n## Summary\r\n\r\nThis PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.\r\n\r\n## Details\r\n\r\nhttps://github.com//issues/171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.\r\n\r\n## Caveats\r\n\r\nUpgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.\r\n\r\nThere are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725. \r\n\r\n- Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in #189725. \r\n\r\n- Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.\r\n\r\n## How to test\r\n\r\nThe simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below\r\n\r\n- Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag\r\n- Run Kibana locally\r\n- Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e`\r\n- Patch the installed rule by running a query below\r\n\r\n```bash\r\ncurl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H \"elastic-api-version: 2023-10-31\" -d '{\"rule_id\":\"2a692072-d78d-42f3-a48a-775677d79c4e\",\"version\":1,\"alert_suppression\":{\"group_by\":[\"host.name\"]}}' http://localhost:5601/kbn/api/detection_engine/rules\r\n```\r\n\r\n- Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button\r\n\r\n## Screenshots\r\n\r\nCustom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)\r\n\r\n![image](https://github.com/user-attachments/assets/86015d5b-e252-4d0b-9aa3-fc14679a493b)\r\n\r\nMachine learning prebuilt rule with a diff in alert suppression\r\n\r\n![image](https://github.com/user-attachments/assets/210246cd-27fd-4976-befc-dee023101ec9)\r\n\r\nThreshold prebuilt rule\r\n\r\n![image](https://github.com/user-attachments/assets/44b0c1bc-4134-4d58-bd9a-e8e2d4c50802)","sha":"06986e4a86a0fa3c3951fcb6b2ba34ebe2769820","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:prev-minor","v8.17.0"],"number":198673,"url":"https://github.com/elastic/kibana/pull/198673","mergeCommit":{"message":"[Security Solution] Add Alert Suppression editable component (#198673)\n\n**Partially addresses:** https://github.com/elastic/kibana/issues/171520\r\n\r\n## Summary\r\n\r\nThis PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.\r\n\r\n## Details\r\n\r\nhttps://github.com//issues/171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.\r\n\r\n## Caveats\r\n\r\nUpgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.\r\n\r\nThere are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725. \r\n\r\n- Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in #189725. \r\n\r\n- Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.\r\n\r\n## How to test\r\n\r\nThe simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below\r\n\r\n- Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag\r\n- Run Kibana locally\r\n- Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e`\r\n- Patch the installed rule by running a query below\r\n\r\n```bash\r\ncurl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H \"elastic-api-version: 2023-10-31\" -d '{\"rule_id\":\"2a692072-d78d-42f3-a48a-775677d79c4e\",\"version\":1,\"alert_suppression\":{\"group_by\":[\"host.name\"]}}' http://localhost:5601/kbn/api/detection_engine/rules\r\n```\r\n\r\n- Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button\r\n\r\n## Screenshots\r\n\r\nCustom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)\r\n\r\n![image](https://github.com/user-attachments/assets/86015d5b-e252-4d0b-9aa3-fc14679a493b)\r\n\r\nMachine learning prebuilt rule with a diff in alert suppression\r\n\r\n![image](https://github.com/user-attachments/assets/210246cd-27fd-4976-befc-dee023101ec9)\r\n\r\nThreshold prebuilt rule\r\n\r\n![image](https://github.com/user-attachments/assets/44b0c1bc-4134-4d58-bd9a-e8e2d4c50802)","sha":"06986e4a86a0fa3c3951fcb6b2ba34ebe2769820"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198673","number":198673,"mergeCommit":{"message":"[Security Solution] Add Alert Suppression editable component (#198673)\n\n**Partially addresses:** https://github.com/elastic/kibana/issues/171520\r\n\r\n## Summary\r\n\r\nThis PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.\r\n\r\n## Details\r\n\r\nhttps://github.com//issues/171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.\r\n\r\n## Caveats\r\n\r\nUpgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.\r\n\r\nThere are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725. \r\n\r\n- Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in #189725. \r\n\r\n- Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.\r\n\r\n## How to test\r\n\r\nThe simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below\r\n\r\n- Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag\r\n- Run Kibana locally\r\n- Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e`\r\n- Patch the installed rule by running a query below\r\n\r\n```bash\r\ncurl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H \"elastic-api-version: 2023-10-31\" -d '{\"rule_id\":\"2a692072-d78d-42f3-a48a-775677d79c4e\",\"version\":1,\"alert_suppression\":{\"group_by\":[\"host.name\"]}}' http://localhost:5601/kbn/api/detection_engine/rules\r\n```\r\n\r\n- Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button\r\n\r\n## Screenshots\r\n\r\nCustom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)\r\n\r\n![image](https://github.com/user-attachments/assets/86015d5b-e252-4d0b-9aa3-fc14679a493b)\r\n\r\nMachine learning prebuilt rule with a diff in alert suppression\r\n\r\n![image](https://github.com/user-attachments/assets/210246cd-27fd-4976-befc-dee023101ec9)\r\n\r\nThreshold prebuilt rule\r\n\r\n![image](https://github.com/user-attachments/assets/44b0c1bc-4134-4d58-bd9a-e8e2d4c50802)","sha":"06986e4a86a0fa3c3951fcb6b2ba34ebe2769820"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
…#198673) **Partially addresses:** elastic#171520 ## Summary This PR adds is built on top of elastic#193828 and elastic#196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow. ## Details elastic#171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case. ## Caveats Upgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade. There are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in elastic#189725. - Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in elastic#189725. - Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown. ## How to test The simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below - Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag - Run Kibana locally - Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e` - Patch the installed rule by running a query below ```bash curl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H "elastic-api-version: 2023-10-31" -d '{"rule_id":"2a692072-d78d-42f3-a48a-775677d79c4e","version":1,"alert_suppression":{"group_by":["host.name"]}}' http://localhost:5601/kbn/api/detection_engine/rules ``` - Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button ## Screenshots Custom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types) ![image](https://github.com/user-attachments/assets/86015d5b-e252-4d0b-9aa3-fc14679a493b) Machine learning prebuilt rule with a diff in alert suppression ![image](https://github.com/user-attachments/assets/210246cd-27fd-4976-befc-dee023101ec9) Threshold prebuilt rule ![image](https://github.com/user-attachments/assets/44b0c1bc-4134-4d58-bd9a-e8e2d4c50802)
…#198673) **Partially addresses:** elastic#171520 ## Summary This PR adds is built on top of elastic#193828 and elastic#196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow. ## Details elastic#171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case. ## Caveats Upgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade. There are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in elastic#189725. - Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in elastic#189725. - Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown. ## How to test The simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below - Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag - Run Kibana locally - Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e` - Patch the installed rule by running a query below ```bash curl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H "elastic-api-version: 2023-10-31" -d '{"rule_id":"2a692072-d78d-42f3-a48a-775677d79c4e","version":1,"alert_suppression":{"group_by":["host.name"]}}' http://localhost:5601/kbn/api/detection_engine/rules ``` - Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button ## Screenshots Custom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types) ![image](https://github.com/user-attachments/assets/86015d5b-e252-4d0b-9aa3-fc14679a493b) Machine learning prebuilt rule with a diff in alert suppression ![image](https://github.com/user-attachments/assets/210246cd-27fd-4976-befc-dee023101ec9) Threshold prebuilt rule ![image](https://github.com/user-attachments/assets/44b0c1bc-4134-4d58-bd9a-e8e2d4c50802)
Partially addresses: #171520
Summary
This PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.
Details
#171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.
Caveats
Upgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.
There are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725.
Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in [Security Solution][Detection Engine] Adds support for suppressing EQL sequence alerts #189725.
Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.
How to test
The simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below
prebuiltRulesCustomizationEnabled
feature flagPotential Code Execution via Postgresql
with rule_id2a692072-d78d-42f3-a48a-775677d79c4e
Detection Rules (SIEM)
Page ->Rule Updates
-> click onPotential Code Execution via Postgresql
rule -> expandEQL Query
to see EQL Query -> pressEdit
buttonScreenshots
Custom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)
Machine learning prebuilt rule with a diff in alert suppression
Threshold prebuilt rule