-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Allow users to edit required_fields field for custom rules #173594
Comments
Yes, we have to think about the design here. The
so basically a collection of objects with 3 properties:
so it's basically a great majority of
So the design could look something similar to Reference URL: |
@jpdjere I was thinking more something similar to alert suppression setup interface. |
Check the suppression component. Question: what to do if the field is mapped with different types in different indexes? Does the suppression component "know" if field is ecs-compliant and what type does it have? |
@approksiu @ARWNightingale Basically, we call the [
{
"name": "@timestamp",
"searchable": true,
"type": "date",
"aggregatable": true,
"esTypes": [
"date"
]
},
{
"name": "_id",
"searchable": true,
"type": "string",
"aggregatable": false,
"esTypes": [
"_id"
]
},
// [... more fields]
{
"name": "agent",
"searchable": true,
"type": "conflict",
"aggregatable": false,
"esTypes": [
"text",
"object"
],
"conflictDescriptions": {
"text": [
"logs-ti_1"
],
"object": [
".ds-auditbeat-8.3.3-2024.01.15-000001",
".ds-packetbeat-8.3.3-2024.01.15-000001"
]
}
},
{
"name": "agent.build.original",
"searchable": true,
"type": "string",
"aggregatable": true,
"esTypes": [
"keyword"
]
},
// [... thousands fields more]
] So we can prepopulate a dropdown field showing the fields name, and when the user selects it, we can prepopulate another dropdown to its right that contains the possible types for the fields, which can get get from each field's The only missing data here is whether the field is ECS compliant or not. I don't think we need to display this to the user, we can calculate it on our side and save it without the user knowing about it. To calculate it, we can use the |
@jpdjere awesome! I like that we can define if the field is ecs-compliant and not have the user fill it in. I assume if one type option is available - it will be automatically selected for the user. |
Yes, once the user chooses a required field, let's preselect the first available option in the |
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Hey @approksiu and @ARWNightingale. I have updated the sandbox with latest Required Fields changes. Would you mind taking a look and providing feedback for it? Sandbox: https://nikitaindik-pr-180682-editable-required-fields.kbndev.co/kbn |
@approksiu @ARWNightingale Here's some additional info about the feature and testing it in the sandbox. The basic flow goes like this: first you specify your index patterns (or a data view), then you can select required fields for these index patterns. I have created two indices in the sandbox deployment:
Since for field WarningsIf a field that is not present in the index pattern is selected, you will see a warning message.
In any of these cases, you'll see a general warning message above the form section. And then also a more specific warning message next to the field that is causing the issue. The warnings won't prevent the user to submit the form. ESQL and ML rulesHere's how available dropdown options are determined for different rule types: For all rule types except ESQL and ML, we take the index patterns specified by the user and fetch thier mappings (fields and their types). Then we use these fields and types to populate the dropdowns. For ESQL rules we parse index patterns out of the query since there's no explicit index pattern form field. We then fetch the mappings for these index patterns and use them to populate the dropdowns. For ML rules, we don't show "required fields" at all. ML rules are a special case.
Sample queriesHere are a few sample queries that can be used to test the feature: |
@nikitaindik this looks good to me. |
Thanks @nikitaindik ! Looks good! |
@vgomez-el Please start exploratory testing when you can |
Hey @joepeeples! I've created a ticket for documenting the UI and API changes: elastic/security-docs#5131 |
@banderror Just to let you know, I started the exploratory testing at the end of the week, but after asking @nikitaindik about some doubts, he told me that there are some last minute changes he needs to work in and push into the branch. I will delay the feature exploratory testing until those changes are finished and pushed to the branch/environment to avoid duplicating testing efforts. |
Hey team! Here's what we have left to do for Required Fields:
Current Serverless release target is Monday, May 6th. But given the remaining work, we might need to aim for the following release on May 13th. This means we would need to complete the review, testing and the docs by the end of the next week. Does moving the release to May 13th work for everyone? If it does, I'll update the dates in this issue and in the docs issue. Please reply below. Thanks! |
@nikitaindik Moving it to the next release works for me. Just let me know when the environment is ready and I will perform the exploratory testing over the feature |
Hey team! Since I'm still working on the PR comments, Required Fields won't make it into the 13th May Serverless release. Rescheduling it to May 20th. I have updated the date in this ticket and in the API docs ticket. |
Finished exploratory testing, looking good! Great work 👍 |
@nikitaindik acceptance testing done. Looks good. Thank you! |
…tom rules (#180682) **Resolves: #173594 **Flaky test runner:** https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5915 ## Summary This PR adds an ability to add and edit custom rule's required fields. "Required fields" is an optional field that shows the user which Elasticsearch fields are needed for the rule to run properly. The values in "required fields" don't affect rule execution in any way. It's purely documentational, similar to "setup guide" and "investigation guide". This functionality is added to both rule creation and rule editing screens. It's available for all rule types except ML. <img width="650" alt="Schermafbeelding 2024-05-07 om 12 28 50" src="https://github.com/elastic/kibana/assets/15949146/70ed052e-3e59-413e-80de-73146225f75a"> ## Details The basic flow goes like this: first you specify your index patterns (or a data view), then you can set required fields for these index patterns. Once a user adds a required field and chooses its name, he can then choose its type from the dropdown. The first available type for the field name selected automatically. User can also add their own custom names and types. ### Warnings If a field that is not present in the selected index pattern, you will see a warning message. This can happen in the following cases: - You have specified an index pattern, selected a required field from this index pattern, and then removed this index pattern. - The index doesn't yet exist. For example, you have installed a prebuilt rule but the data for it hasn't been ingested yet, so there's no index yet. - The index was removed. - The mappings for the index were changed and the field is no longer present. In any of these cases, you'll see a general warning message above the form section. And then also a more specific warning message next to the field that is causing the issue. ### ESQL and ML rules Here's how available dropdown options are determined for different rule types: For all rule types except ESQL and ML, we take the index patterns specified by the user and fetch their mappings. Then we use these fields and types to populate the dropdowns. For ESQL rules we parse index patterns out of the query since there's no explicit index pattern form field. We then fetch the mappings for these index patterns and use them to populate the dropdowns. For ML rules, we don't show "required fields" at all. ML rules are a special case. 1. The concept of "required fields" is sort of handled during creation of the ML job itself, where the user specifies the fields that are required for the job to run. 2. In the ML rule creation/editing interface, we don't display the index patterns a rule operates on. So, even if we allowed specifying required fields, the user would need to check the ML job details to see the index patterns the job uses. 3. The ML job dropdown includes both existing and not-yet-created jobs. We can't get index patterns for jobs that don't exist yet, so we can't fill the dropdowns with fields and types. ## Screenshots <img width="628" alt="screen1_" src="https://github.com/elastic/kibana/assets/15949146/aade141f-8285-44c6-8c56-611ba1a9f17b"> <img width="601" alt="screen2_" src="https://github.com/elastic/kibana/assets/15949146/b44fb254-c254-44b8-9600-45b47f29a421">
Hey folks! 🎉 The main Required Fields PR and the API docs PR, have both been merged. Woohoo! This means the feature is going into the Monday, the 20th Serverless release! 🚀 High-fives to everyone who helped with the review and testing! 🙌💖 |
The feature went live in Serverless on Tue, May 21st 🎉 |
…ed integrations and required fields from the original rule (#191065) **Fixes: #190628 **Related to:** #173595, #173594 ## Summary As stated in the bug ticket, when duplicating a prebuilt rule, the "Related Integrations" and "Required Fields" values should be inherited from the original rule, as it was specified in the Acceptance Criteria for #173595 and #173594. This PR: - Removes the logic that resets these fields to empty arrays for duplicated prebuilt rules - we needed this logic in the past because these fields were not editable in the UI, but we don't need it anymore. - Updates the corresponding unit tests. ## Screenshots These screenshots were taken after introducing the fixes. **Original prebuilt rule:** <img width="1463" alt="Screenshot_2024-08-23_at_13_25_07" src="https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119"> **Duplicated prebuilt rule:** <img width="1469" alt="Screenshot_2024-08-23_at_13_25_43" src="https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b"> ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…ed integrations and required fields from the original rule (elastic#191065) **Fixes: elastic#190628 **Related to:** elastic#173595, elastic#173594 ## Summary As stated in the bug ticket, when duplicating a prebuilt rule, the "Related Integrations" and "Required Fields" values should be inherited from the original rule, as it was specified in the Acceptance Criteria for elastic#173595 and elastic#173594. This PR: - Removes the logic that resets these fields to empty arrays for duplicated prebuilt rules - we needed this logic in the past because these fields were not editable in the UI, but we don't need it anymore. - Updates the corresponding unit tests. ## Screenshots These screenshots were taken after introducing the fixes. **Original prebuilt rule:** <img width="1463" alt="Screenshot_2024-08-23_at_13_25_07" src="https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119"> **Duplicated prebuilt rule:** <img width="1469" alt="Screenshot_2024-08-23_at_13_25_43" src="https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b"> ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit b144c05) # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts
…y related integrations and required fields from the original rule (#191065) (#191493) # Backport This will backport the following commits from `main` to `8.15`: - [[Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065)](#191065) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Georgii Gorbachev","email":"georgii.gorbachev@elastic.co"},"sourceCommit":{"committedDate":"2024-08-26T13:42:52Z","message":"[Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065)\n\n**Fixes: https://github.com/elastic/kibana/issues/190628**\r\n**Related to:** https://github.com/elastic/kibana/issues/173595,\r\nhttps://github.com/elastic/kibana/issues/173594\r\n\r\n## Summary\r\n\r\nAs stated in the bug ticket, when duplicating a prebuilt rule, the\r\n\"Related Integrations\" and \"Required Fields\" values should be inherited\r\nfrom the original rule, as it was specified in the Acceptance Criteria\r\nfor #173595 and\r\nhttps://github.com//issues/173594.\r\n\r\nThis PR:\r\n\r\n- Removes the logic that resets these fields to empty arrays for\r\nduplicated prebuilt rules - we needed this logic in the past because\r\nthese fields were not editable in the UI, but we don't need it anymore.\r\n- Updates the corresponding unit tests.\r\n\r\n## Screenshots\r\n\r\nThese screenshots were taken after introducing the fixes.\r\n\r\n**Original prebuilt rule:**\r\n\r\n<img width=\"1463\" alt=\"Screenshot_2024-08-23_at_13_25_07\"\r\nsrc=\"https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119\">\r\n\r\n**Duplicated prebuilt rule:**\r\n\r\n<img width=\"1469\" alt=\"Screenshot_2024-08-23_at_13_25_43\"\r\nsrc=\"https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b\">\r\n\r\n### Checklist\r\n\r\n- [ ]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"b144c05e8f39f28dd9551b7c62daa01cfa1d2cd5","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","impact:medium","Team:Detections and Resp","Team: SecuritySolution","Feature:Rule Management","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","v8.16.0","v8.15.1"],"number":191065,"url":"https://github.com/elastic/kibana/pull/191065","mergeCommit":{"message":"[Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065)\n\n**Fixes: https://github.com/elastic/kibana/issues/190628**\r\n**Related to:** https://github.com/elastic/kibana/issues/173595,\r\nhttps://github.com/elastic/kibana/issues/173594\r\n\r\n## Summary\r\n\r\nAs stated in the bug ticket, when duplicating a prebuilt rule, the\r\n\"Related Integrations\" and \"Required Fields\" values should be inherited\r\nfrom the original rule, as it was specified in the Acceptance Criteria\r\nfor #173595 and\r\nhttps://github.com//issues/173594.\r\n\r\nThis PR:\r\n\r\n- Removes the logic that resets these fields to empty arrays for\r\nduplicated prebuilt rules - we needed this logic in the past because\r\nthese fields were not editable in the UI, but we don't need it anymore.\r\n- Updates the corresponding unit tests.\r\n\r\n## Screenshots\r\n\r\nThese screenshots were taken after introducing the fixes.\r\n\r\n**Original prebuilt rule:**\r\n\r\n<img width=\"1463\" alt=\"Screenshot_2024-08-23_at_13_25_07\"\r\nsrc=\"https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119\">\r\n\r\n**Duplicated prebuilt rule:**\r\n\r\n<img width=\"1469\" alt=\"Screenshot_2024-08-23_at_13_25_43\"\r\nsrc=\"https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b\">\r\n\r\n### Checklist\r\n\r\n- [ ]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"b144c05e8f39f28dd9551b7c62daa01cfa1d2cd5"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/191065","number":191065,"mergeCommit":{"message":"[Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065)\n\n**Fixes: https://github.com/elastic/kibana/issues/190628**\r\n**Related to:** https://github.com/elastic/kibana/issues/173595,\r\nhttps://github.com/elastic/kibana/issues/173594\r\n\r\n## Summary\r\n\r\nAs stated in the bug ticket, when duplicating a prebuilt rule, the\r\n\"Related Integrations\" and \"Required Fields\" values should be inherited\r\nfrom the original rule, as it was specified in the Acceptance Criteria\r\nfor #173595 and\r\nhttps://github.com//issues/173594.\r\n\r\nThis PR:\r\n\r\n- Removes the logic that resets these fields to empty arrays for\r\nduplicated prebuilt rules - we needed this logic in the past because\r\nthese fields were not editable in the UI, but we don't need it anymore.\r\n- Updates the corresponding unit tests.\r\n\r\n## Screenshots\r\n\r\nThese screenshots were taken after introducing the fixes.\r\n\r\n**Original prebuilt rule:**\r\n\r\n<img width=\"1463\" alt=\"Screenshot_2024-08-23_at_13_25_07\"\r\nsrc=\"https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119\">\r\n\r\n**Duplicated prebuilt rule:**\r\n\r\n<img width=\"1469\" alt=\"Screenshot_2024-08-23_at_13_25_43\"\r\nsrc=\"https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b\">\r\n\r\n### Checklist\r\n\r\n- [ ]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"b144c05e8f39f28dd9551b7c62daa01cfa1d2cd5"}},{"branch":"8.15","label":"v8.15.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168
Summary
We need to expose the required_fileds field in the rule edit page and allow editing it.
The required_fields is an optional field.
Background
We want to allow users to easily specify the prerequisites for their custom rules.
User story
Acceptance criteria
Release progress
Planned release date in Serverless: May 20th, 2024.
Planned release date in ESS:
v8.15.0
.The text was updated successfully, but these errors were encountered: