[Security] Adds field mapping support to rule creation Part I #70288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spong
added
release_note:enhancement
Team:SIEM
v8.0.0
v7.9.0
Feature:Detection Rules
…tion in edit mode
|
Pinging @elastic/siem (Team:SIEM) |
| @@ -179,18 +203,65 @@ export type Name = t.TypeOf<typeof name>; | |||
| export const nameOrUndefined = t.union([name, t.undefined]); | |||
| export type NameOrUndefined = t.TypeOf<typeof nameOrUndefined>; | |||
|
|
|||
| export const operator = t.keyof({ | |||
| equals: null, | |||
There was a problem hiding this comment.
Nit: You can use maybe: OperatorEnum['EQUALS']: null here. That would reduce one area of manual typing.
| @@ -1539,6 +1560,9 @@ describe('add prepackaged rules schema', () => { | |||
| const message = pipe(checked, foldLeftRight); | |||
| expect(getPaths(left(message.errors))).toEqual([]); | |||
| const expected: AddPrepackagedRulesSchemaDecoded = { | |||
| author: [], | |||
| severity_mapping: [], | |||
| risk_score_mapping: [], | |||
| rule_id: 'rule-1', | |||
| description: 'some description', | |||
| from: 'now-5m', | |||
There was a problem hiding this comment.
You should add a test where you exercise each of these array values to convince yourself they will work as well as one where you have an invalid data type such as a string instead of an array or a number instead of a string.
Then basically cut across the other schemas and add the "proof" that it validates to each of those.
There was a problem hiding this comment.
Thanks! I've noted and will include those in Part II!
| @@ -1591,6 +1624,9 @@ describe('create rules schema', () => { | |||
| const message = pipe(checked, foldLeftRight); | |||
| expect(getPaths(left(message.errors))).toEqual([]); | |||
| const expected: CreateRulesSchemaDecoded = { | |||
| author: [], | |||
| severity_mapping: [], | |||
| risk_score_mapping: [], | |||
| rule_id: 'rule-1', | |||
| description: 'some description', | |||
| from: 'now-5m', | |||
There was a problem hiding this comment.
You should add a test where you exercise each of these array values to convince yourself they will work as well as one where you have an invalid data type such as a string instead of an array or a number instead of a string.
Then basically cut across the other schemas and add the "proof" that it validates to each of those.
| @@ -1727,6 +1754,9 @@ describe('import rules schema', () => { | |||
| const message = pipe(checked, foldLeftRight); | |||
| expect(getPaths(left(message.errors))).toEqual([]); | |||
| const expected: ImportRulesSchemaDecoded = { | |||
| author: [], | |||
| severity_mapping: [], | |||
| risk_score_mapping: [], | |||
| rule_id: 'rule-1', | |||
| description: 'some description', | |||
| from: 'now-5m', | |||
There was a problem hiding this comment.
You should add a test where you exercise each of these array values to convince yourself they will work as well as one where you have an invalid data type such as a string instead of an array or a number instead of a string.
Then basically cut across the other schemas and add the "proof" that it validates to each of those.
| @@ -1531,6 +1558,9 @@ describe('update rules schema', () => { | |||
| const message = pipe(checked, foldLeftRight); | |||
| expect(getPaths(left(message.errors))).toEqual([]); | |||
| const expected: UpdateRulesSchemaDecoded = { | |||
| author: [], | |||
| severity_mapping: [], | |||
| risk_score_mapping: [], | |||
| rule_id: 'rule-1', | |||
| description: 'some description', | |||
| from: 'now-5m', | |||
There was a problem hiding this comment.
You should add a test where you exercise each of these array values to convince yourself they will work as well as one where you have an invalid data type such as a string instead of an array or a number instead of a string.
Then basically cut across the other schemas and add the "proof" that it validates to each of those.
| t.identity | ||
| ); | ||
|
|
||
| export type DefaultRiskScoreMappingArrayC = typeof DefaultRiskScoreMappingArray; |
There was a problem hiding this comment.
You should copy the boiler plate code from one of the others and add it here. Shouldn't take you long.
| t.identity | ||
| ); | ||
|
|
||
| export type DefaultSeverityMappingArrayC = typeof DefaultSeverityMappingArray; |
There was a problem hiding this comment.
You should copy the boiler plate code from one of the others and add it here. Shouldn't take you long.
| 'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldAuthorHelpText', | ||
| { | ||
| defaultMessage: | ||
| 'Type one or more author for this rule. Press enter after each author to add a new one.', |
There was a problem hiding this comment.
Did you want author with an s like:
Type one or more authors for this rule.
| @@ -377,13 +383,18 @@ describe('helpers', () => { | |||
| }; | |||
| const result: AboutStepRuleJson = formatAboutStepData(mockStepData); | |||
| const expected = { | |||
There was a problem hiding this comment.
Can we type this with the Schema at this point?
const expected: RuleSchemaNot a biggie but would a nice win to know that we have more compile time checks when we change things.
| @@ -414,12 +426,17 @@ describe('helpers', () => { | |||
| }; | |||
| const result: AboutStepRuleJson = formatAboutStepData(mockStepData); | |||
| const expected = { | |||
| author: ['Elastic'], | |||
There was a problem hiding this comment.
Can we type this with the Schema at this point?
const expected: RuleSchemaNot a biggie but would a nice win to know that we have more compile time checks when we change things.
| @@ -481,13 +499,18 @@ describe('helpers', () => { | |||
| }; | |||
| const result: AboutStepRuleJson = formatAboutStepData(mockStepData); | |||
| const expected = { | |||
There was a problem hiding this comment.
Can we type this with the Schema at this point?
const expected: RuleSchemaNot a biggie but would a nice win to know that we have more compile time checks when we change things.
| RuleNameOverride, | ||
| SeverityMapping, | ||
| TimestampOverride, | ||
| } from '../../../../../common/detection_engine/schemas/common/schemas'; |
There was a problem hiding this comment.
NICE! The common schemas are beginning to make their appearance on the front end.
There was a problem hiding this comment.
Slowly but surely! I didn't want to make the full-swap as part of this PR in case there was any collateral damage, but may inch a bit closer in Part II now that I understand how all our types/schemas are working a bit better 🙂
| @@ -388,6 +394,7 @@ export const getResult = (): RuleAlertType => ({ | |||
| ], | |||
| }, | |||
| ], | |||
| timestampOverride: undefined, | |||
| references: ['http://www.example.com', 'https://ww.example.com'], | |||
| note: '# Investigative notes', | |||
| version: 1, | |||
There was a problem hiding this comment.
Doesn't have to be this PR, but we should move towards getting more code out of mocks and as close to their respective schema files where possible or just move them into a closely named file_name.mock.ts somewhere.
Just keep it in mind as you're going through here that I think it's more Kibana core like to move things out of the mocks folders and it has just been easier to maintain and find mocks that way IMHO.
x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts
Outdated
Show resolved
Hide resolved
| @@ -24,7 +24,7 @@ do { | |||
| -u ${ELASTICSEARCH_USERNAME}:${ELASTICSEARCH_PASSWORD} \ | |||
| -X POST ${KIBANA_URL}${SPACE_URL}/api/detection_engine/rules \ | |||
| -d @${RULE} \ | |||
| | jq .; | |||
| | jq -S .; | |||
There was a problem hiding this comment.
I am fine with this. I think for consistency we should update everywhere but I won't inflict that on this PR. You can leave it here and we will do a sweep through to do a sort on all the fields when doing POST, GET, DELETE, later...
There was a problem hiding this comment.
Whoops -- left that accidentally... 😬 I've made a note to update the rest to use it as well in Part II. Was really helpful in testing/debugging via the CLI scripts -- would like to turn on the linter rule for alphabetizing params too as that caused a bit of fatigue through all of this as well!
| } | ||
| ], | ||
| "timestamp_override": "event.ingested" | ||
| } |
There was a problem hiding this comment.
There's a query called query_with_everything.json that is supposed to be the kitchen sink example FWIW. It's best to update that one with any missing fields and we typically use it to put everything under the sun into it.
x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts
Outdated
Show resolved
Hide resolved
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jul 2, 2020
* master: (46 commits) [Visualize] Add missing advanced settings and custom label for pipeline aggs (elastic#69688) Use dynamic: false for config saved object mappings (elastic#70436) [Ingest Pipelines] Error messages (elastic#70167) [APM] Show transaction rate per minute on Observability Overview page (elastic#70336) Filter out error when calculating a label (elastic#69934) [Visualizations] Each visType returns its supported triggers (elastic#70177) [Telemetry] Report data shippers (elastic#64935) Reduce SavedObjects mappings for Application Usage (elastic#70475) [Lens] fix dimension label performance issues (elastic#69978) Skip failing endgame tests (elastic#70548) [SIEM] Reenabling Cypress tests (elastic#70397) [SIEM][Security Solution][Endpoint] Endpoint Artifact Manifest Management + Artifact Download and Distribution (elastic#67707) [Security] Adds field mapping support to rule creation (elastic#70288) SECURITY-ENDPOINT: add fields for events to metadata document (elastic#70491) Fixed assertion in hybrid index pattern test to iterate through indices (elastic#70130) [SIEM][Exceptions] - Exception builder component (elastic#67013) [Ingest Manager] Rename data sources to package configs (elastic#70259) skip suites blocking es snapshot promomotion (elastic#70532) [Metrics UI] Fix asynchronicity and error handling in Snapshot API (elastic#70503) fix export response (elastic#70473) ...
FrankHassanabad
pushed a commit
to FrankHassanabad/kibana
that referenced
this pull request
Jul 2, 2020
## Summary Resolves: elastic#65941, elastic#66317, and `Add support for "building block" alerts` This PR is `Part I` and adds additional fields to the `rules schema` in supporting the ability to map and override fields when generating alerts. A few bookkeeping fields like `license` and `author` have been added as well. The new fields are as follows: ``` ts export interface TheseAreTheNewFields { author: string[]; building_block_type: string; // 'default' license: string; risk_score_mapping: Array< { field: string; operator: string; // 'equals' value: string; } >; rule_name_override: string; severity_mapping: Array< { field: string; operator: string; // 'equals' value: string; severity: string; // 'low' | 'medium' | 'high' | 'critical' } >; timestamp_override: string; } ``` These new fields are exposed as additional settings on the `About rule` section of the Rule Creation UI. ##### Default collapsed view, no severity or risk score override specified: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/86090417-49c0ee80-ba67-11ea-898f-a43af6d9383f.png" /> </p> ##### Severity & risk score override specified: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/86091165-a8d33300-ba68-11ea-86ac-89393a7ca3f5.png" /> </p> ##### Additional fields in Advanced settings: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/86091256-cbfde280-ba68-11ea-9b63-acf2524039bd.png" /> </p> Note: This PR adds the fields to the `Rules Schema`, the `signals index mapping`, and creates the UI for adding these fields during Rule Creation/Editing. The follow-up `Part II` will add the business logic for mapping fields during `rule execution`, and also add UI validation/additional tests. ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md) - [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials - Syncing w/ @benskelker - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios - [x] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist) ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
FrankHassanabad
added a commit
that referenced
this pull request
Jul 2, 2020
…#70603) * [Security] Adds field mapping support to rule creation (#70288) ## Summary Resolves: #65941, #66317, and `Add support for "building block" alerts` This PR is `Part I` and adds additional fields to the `rules schema` in supporting the ability to map and override fields when generating alerts. A few bookkeeping fields like `license` and `author` have been added as well. The new fields are as follows: ``` ts export interface TheseAreTheNewFields { author: string[]; building_block_type: string; // 'default' license: string; risk_score_mapping: Array< { field: string; operator: string; // 'equals' value: string; } >; rule_name_override: string; severity_mapping: Array< { field: string; operator: string; // 'equals' value: string; severity: string; // 'low' | 'medium' | 'high' | 'critical' } >; timestamp_override: string; } ``` These new fields are exposed as additional settings on the `About rule` section of the Rule Creation UI. ##### Default collapsed view, no severity or risk score override specified: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/86090417-49c0ee80-ba67-11ea-898f-a43af6d9383f.png" /> </p> ##### Severity & risk score override specified: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/86091165-a8d33300-ba68-11ea-86ac-89393a7ca3f5.png" /> </p> ##### Additional fields in Advanced settings: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/86091256-cbfde280-ba68-11ea-9b63-acf2524039bd.png" /> </p> Note: This PR adds the fields to the `Rules Schema`, the `signals index mapping`, and creates the UI for adding these fields during Rule Creation/Editing. The follow-up `Part II` will add the business logic for mapping fields during `rule execution`, and also add UI validation/additional tests. ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md) - [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials - Syncing w/ @benskelker - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios - [x] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist) ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process) * Fixed i18n keys Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
spong
changed the title
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/index_management/home_page·ts.Index Management app Home page Component templates renders the component templates tabStandard OutStack TraceKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/doc_level_security_roles·js.security app dls user East should only see EAST docStandard OutStack TraceKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/doc_level_security_roles·js.security app dls user East should only see EAST docStandard OutStack Traceand 3 more failures, only showing the first 3. Build metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
This was referenced Jul 9, 2020
spong
added a commit
that referenced
this pull request
Jul 14, 2020
## Summary Followup to #70288, which includes: - [X] Rule Execution logic for: - [X] Severity Override - [X] Risk Score Override - [X] Rule Name Override - [X] Timestamp Override - [X] Support for toggling display of Building Block Rules: - [X] Main Detections Page - [X] Rule Details Page - [X] Integrates `AutocompleteField` for: - [X] Severity Override - [X] Risk Score Override - [X] Rule Name Override - [X] Timestamp Override - [X] Fixes rehydration of `EditAboutStep` in `Edit Rule` - [X] Fixes `Rule Details` Description rollup Additional followup cleanup: - [ ] Adds risk_score` to `risk_score_mapping` - [ ] Improves field validation - [ ] Disables override fields for ML Rules - [ ] Orders `SeverityMapping` by `severity` on create/update - [ ] Allow unbounded max-signals ### Checklist Delete any items that are not applicable to this PR. - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md) - [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials - Syncing w/ @benskelker - [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios ### For maintainers - [X] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong
added a commit
that referenced
this pull request
Jul 14, 2020
… (#71775) ## Summary Followup to #70288, which includes: - [X] Rule Execution logic for: - [X] Severity Override - [X] Risk Score Override - [X] Rule Name Override - [X] Timestamp Override - [X] Support for toggling display of Building Block Rules: - [X] Main Detections Page - [X] Rule Details Page - [X] Integrates `AutocompleteField` for: - [X] Severity Override - [X] Risk Score Override - [X] Rule Name Override - [X] Timestamp Override - [X] Fixes rehydration of `EditAboutStep` in `Edit Rule` - [X] Fixes `Rule Details` Description rollup Additional followup cleanup: - [ ] Adds risk_score` to `risk_score_mapping` - [ ] Improves field validation - [ ] Disables override fields for ML Rules - [ ] Orders `SeverityMapping` by `severity` on create/update - [ ] Allow unbounded max-signals ### Checklist Delete any items that are not applicable to this PR. - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md) - [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials - Syncing w/ @benskelker - [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios ### For maintainers - [X] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
This was referenced Jul 20, 2020
MindyRS
added
the
Team: SecuritySolution
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Resolves: #65941, #66317, and
Add support for "building block" alertsThis PR is
Part Iand adds additional fields to therules schemain supporting the ability to map and override fields when generating alerts. A few bookkeeping fields likelicenseandauthorhave been added as well. The new fields are as follows:These new fields are exposed as additional settings on the
About rulesection of the Rule Creation UI.Default collapsed view, no severity or risk score override specified:
Severity & risk score override specified:
Additional fields in Advanced settings:
Note: This PR adds the fields to the
Rules Schema, thesignals index mapping, and creates the UI for adding these fields during Rule Creation/Editing. The follow-upPart IIwill add the business logic for mapping fields duringrule execution, and also add UI validation/additional tests.Checklist
Delete any items that are not applicable to this PR.
For maintainers