-
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
[SIEM][Exceptions] - Cleaned up and updated exception list item comment structure #69532
Conversation
…void empty comments
t.array(commentPartial).is, | ||
(input, context): Either<t.Errors, CommentsPartialArray> => | ||
input == null ? t.success([]) : t.array(commentPartial).validate(input, context), | ||
t.array(comments).is, |
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.
NOTE: Previously had the different comment schemas all in one doc, but started to feel really messy. Broke out into their own docs, tests, and mocks.
Pinging @elastic/siem (Team:SIEM) |
@@ -176,7 +176,7 @@ export const updateExceptionListItem = async ({ | |||
|
|||
if (validatedRequest != null) { | |||
try { | |||
const response = await http.fetch<ExceptionListItemSchema>(EXCEPTION_LIST_URL, { | |||
const response = await http.fetch<ExceptionListItemSchema>(EXCEPTION_LIST_ITEM_URL, { |
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.
Thanks for this fix!
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
…nt structure (elastic#69532) ### Summary This PR is a follow up to elastic#68864 . That PR used a partial to differentiate between new and existing comments, this meant that comments could be updated when they shouldn't. It was decided in our discussion of exception list schemas that comments should be append only. This PR assures that's the case, but also leaves it open to editing comments (via API). It checks to make sure that users can only update their own comments.
* master: skip failing suite (elastic#70104) (elastic#70103) [ENDPOINT] Hide the Timeline Flyout while on the Management Pages (elastic#69998) [SIEM][CASE] Persist callout when dismissed (elastic#68372) [SIEM][Exceptions] - Cleaned up and updated exception list item comment structure (elastic#69532) [Maps] remove indexing state from redux (elastic#69765) Add API integration test for deleting data streams. (elastic#70020) renames SIEM to Security Solution (elastic#70070)
…nt structure (#69532) (#70107) ### Summary This PR is a follow up to #68864 . That PR used a partial to differentiate between new and existing comments, this meant that comments could be updated when they shouldn't. It was decided in our discussion of exception list schemas that comments should be append only. This PR assures that's the case, but also leaves it open to editing comments (via API). It checks to make sure that users can only update their own comments.
* master: (59 commits) [Lens] Fix broken test (elastic#70117) [SIEM] Import timeline fix (elastic#65448) [SECURITY SOLUTION][INGEST] UX update for ingest manager edit/create datasource for endpoint (elastic#70079) [Telemetry] Collector Schema (elastic#64942) [Endpoint] Add Endpoint empty states for onboarding (elastic#69626) Hide unused resolver buttons (elastic#70112) [Security] `Investigate in Resolver` Timeline Integration (elastic#70111) [Discover] Improve styling of graphs in sidebar (elastic#69440) [Metrics UI] Fix EuiTheme type issue (elastic#69735) skip failing suite (elastic#70104) (elastic#70103) [ENDPOINT] Hide the Timeline Flyout while on the Management Pages (elastic#69998) [SIEM][CASE] Persist callout when dismissed (elastic#68372) [SIEM][Exceptions] - Cleaned up and updated exception list item comment structure (elastic#69532) [Maps] remove indexing state from redux (elastic#69765) Add API integration test for deleting data streams. (elastic#70020) renames SIEM to Security Solution (elastic#70070) Adding saved_objects_page in OSS (elastic#69900) [Lens] Use accordion menus in field list for available and empty fields (elastic#68871) Dynamic uiActions & license support (elastic#68507) [SIEM] Update readme for timeline apis (elastic#67038) ...
…bana into alerting/consumer-based-rbac * 'alerting/consumer-based-rbac' of github.com:gmmorris/kibana: (25 commits) [Lens] Fix broken test (elastic#70117) [SIEM] Import timeline fix (elastic#65448) [SECURITY SOLUTION][INGEST] UX update for ingest manager edit/create datasource for endpoint (elastic#70079) [Telemetry] Collector Schema (elastic#64942) [Endpoint] Add Endpoint empty states for onboarding (elastic#69626) Hide unused resolver buttons (elastic#70112) [Security] `Investigate in Resolver` Timeline Integration (elastic#70111) [Discover] Improve styling of graphs in sidebar (elastic#69440) [Metrics UI] Fix EuiTheme type issue (elastic#69735) skip failing suite (elastic#70104) (elastic#70103) [ENDPOINT] Hide the Timeline Flyout while on the Management Pages (elastic#69998) [SIEM][CASE] Persist callout when dismissed (elastic#68372) [SIEM][Exceptions] - Cleaned up and updated exception list item comment structure (elastic#69532) [Maps] remove indexing state from redux (elastic#69765) Add API integration test for deleting data streams. (elastic#70020) renames SIEM to Security Solution (elastic#70070) Adding saved_objects_page in OSS (elastic#69900) [Lens] Use accordion menus in field list for available and empty fields (elastic#68871) Dynamic uiActions & license support (elastic#68507) [SIEM] Update readme for timeline apis (elastic#67038) ...
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
This PR is a follow up to #68864 . That PR used a partial to differentiate between new and existing comments, this meant that comments could be updated when they shouldn't. It was decided in our discussion of exception list schemas that comments should be append only. This PR assures that's the case, but also leaves it open to editing comments (via API). It checks to make sure that users can only update their own comments.
Created 3 different schemas for comments, during creation, should only accept comments of following schema:
Saved comments must be of the following schema:
Much of the added logic deals with updating comments (PUT), the exception list item
comments
can include a mix of the two above schemas, but the added logic ensures that:updated_at
andupdated_by
created_at
, andcreated_by
Looking over the tests is helpful in understanding the logic, and may help you help me find any logic I've missed. 😄
I chose to separate out the different schemas into their own files to maintain readability and maintainability. It looked a little icky when putting everything into one file.
Testing
I added unit tests to confirm the logic, but also used the existing scripts.
To turn on lists plugin - in kibana.dev.yml
Use the scripts in
x-pack/plugins/lists/server/scripts
to create some sample exception lists and items. You can use the following:./post_exception_list.sh
./post_exception_list_item.sh
./update_exception_list_item.sh
You can update the scripts to try different edge cases (deleting comments, updating different user comments, etc).
If you want to view the comments in the UI:
./find_exception_lists.sh
to get the id of the two lists you createdExceptionsViewer
component inx-pack/plugins/security_solution/public/alerts/pages/detection_engine/rules/details/index.tsx
to something like the following:Navigate to the rules details page and click on the 'Exceptions' tab. Voila!
Checklist
Delete any items that are not applicable to this PR.
For maintainers
(It is a breaking change, however this API is not yet public and behind feature flag, only devs working on this feature would be affected)