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

[SIEM][Exceptions] - ExceptionsViewer UI component part 2 #68294

Merged
merged 35 commits into from
Jun 10, 2020

Conversation

yctercero
Copy link
Contributor

@yctercero yctercero commented Jun 4, 2020

Summary

This PR is a follow up to #68027 . It brings it all together to complete the exceptions viewer component. This component is meant to display all exception items and allow a user to create, edit, delete, and search these exception items.

  • Moves ExceptionItem (from part 1) into its own folder
  • Adds exceptions_viewer_header component that includes the search, list toggle, and add exception buttons
  • Adds actual ExceptionViewer component
  • Updates the useExceptionList hook refresh function logic. Noticed that the previous version was creating some issues

Note

  • There's a bit of funkiness right now as the exception list hook returns the existing exception list schema and the viewer is relying on the new exception list schema. Not too big a deal, should be easy cleanup once new schemas are ready. You'll see some TODOs in areas I'll have to update.
  • There's some filler code for where the add and edit exception modal logic will go. It's likely some of the logic around that will need to be updated.
  • Made an assumption that rule.exceptionLists would be an array of { id: '', type: 'detections', namespaceType: 'single' }. I can update once that final structure is figured out as well.
  • The file x-pack/plugins/security_solution/public/alerts/pages/detection_engine/rules/details/index.tsx is due for a refactor, as this PR is already getting big, should be a follow up

To Do

  • add unit tests for main ExceptionViewer component
  • cleanup after new schemas are in
  • update logic to allow 3 views (all items, detection items, endpoint items)
  • update empty page title and description text
  • update loading states
  • determine best pagination user experience
  • should the KQL search bar be hidden for the Exceptions tab?
  • add transition animation on item delete
  • test pagination once _find endpoint updated

Testing

To turn on lists plugin - in kibana.dev.yml

# Enable lists feature
xpack.lists.enabled: true
xpack.lists.listIndex: '.lists-yara'
xpack.lists.listItemIndex: '.items-yara'

Use the scripts in x-pack/plugins/lists/server/scripts to create some sample exception lists and items. You can use the following:

  1. Create detection type list ./post_exception_list.sh ./exception_lists/new/exception_list_detection.json
  2. Create detection type list items ./post_exception_list_item.sh ./exception_lists/new/exception_list_item_detection_auto_id.json - this script auto generates the item_id so you can run it as many times as you like to create multiple items associated with the list generated in step 1
  3. Create endpoint list ./post_exception_list.sh
  4. Create endpoint type list items ./post_exception_list_item.sh ./exception_lists/new/exception_list_item_auto_id.json - this script auto generates the item_id so you can run it as many times as you like to create multiple items associated with the list generated in step 3
  5. Run ./find_exception_lists.sh to get the id of the two lists you created
  6. Update the ExceptionsViewer component in x-pack/plugins/security_solution/public/alerts/pages/detection_engine/rules/details/index.tsx to something like the following:
 <ExceptionsViewer
       commentsAccordionId={'someId'}
       exceptionLists={[
         {
            id: [DETECTION_LIST_ID],  // `id` not `list_id`
            type: 'detection',
            namespaceType: 'single',
         },
         {
            id: [ENDPOINT_LIST_ID],  // `id` not `list_id`
             type: 'endpoint',
             namespaceType: 'single',
           },
         ]}  />

Navigate to the rules details page and click on the 'Exceptions' tab. Voila!

Screenshots

Empty

Screen Shot 2020-06-08 at 5 20 08 PM

Items exist

Screen Shot 2020-06-08 at 5 19 00 PM

Checklist

@@ -5,6 +5,8 @@
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: As mentioned in the 'part 1' PR, these types will get cleaned once the exceptions structure is updated.

Comment on lines +14 to +23
export interface ExceptionsApi {
deleteExceptionItem: (arg: ApiCallMemoProps) => Promise<void>;
deleteExceptionList: (arg: ApiCallMemoProps) => Promise<void>;
getExceptionItem: (
arg: ApiCallMemoProps & { onSuccess: (arg: ExceptionListItemSchema) => void }
) => Promise<void>;
getExceptionList: (
arg: ApiCallMemoProps & { onSuccess: (arg: ExceptionListSchema) => void }
) => Promise<void>;
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice way of exposing the list API's! Makes for a clean separation of concerns between usage in hooks, and exposing the plugin's API as a whole 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, the other way felt strange and saw this pattern elsewhere.

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 and tested locally, and also performed a code review -- looking good here! 😀 Thanks for the continued cleanup, updates to tests, and new auto_id exception list json's for testing -- they really help getting set up and quickly testing out the UI!

Also.... 👏 👏 👏 on leveraging storybooks! We haven't used them much on the SIEM side, but I've heard great things. It was nice to be able to click through and test all the different states.

LGTM! Thanks @yctercero! 👍 🚀

🎉 🎉

@yctercero
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@yctercero yctercero merged commit 8095856 into elastic:master Jun 10, 2020
yctercero added a commit to yctercero/kibana that referenced this pull request Jun 10, 2020
)

### Summary 

This PR is a follow up to elastic#68027 . It brings it all together to complete the exceptions viewer component. This component is meant to display all exception items and allow a user to create, edit, delete, and search these exception items.

- Moves ExceptionItem (from part 1) into its own folder
- Adds exceptions_viewer_header component that includes the search, list toggle, and add exception buttons
- Adds actual ExceptionViewer component
- Updates the useExceptionList hook refresh function logic. Noticed that the previous version was creating some issues
yctercero added a commit that referenced this pull request Jun 10, 2020
…68729)

### Summary 

This PR is a follow up to #68027 . It brings it all together to complete the exceptions viewer component. This component is meant to display all exception items and allow a user to create, edit, delete, and search these exception items.

- Moves ExceptionItem (from part 1) into its own folder
- Adds exceptions_viewer_header component that includes the search, list toggle, and add exception buttons
- Adds actual ExceptionViewer component
- Updates the useExceptionList hook refresh function logic. Noticed that the previous version was creating some issues
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 10, 2020
* master: (22 commits)
  Partial revert of "Sync Kerberos + Anonymous access tests with the latest `security/_authenticate` API (user roles now include roles of anonymous user)." (elastic#68624)
  adapt some snapshot test (elastic#68489)
  [APM] Service maps - Fix missing ML status for services with jobs but no anomalies (elastic#68486)
  [skip test] apis Kerberos security Kerberos authentication finishing SPNEGO should properly set cookie and authenticate user
  [SIEM][Exceptions] - ExceptionsViewer UI component part 2 (elastic#68294)
  Surface data streams in Index Management. (elastic#67806)
  Fix edit datasource not working following changes in elastic#67234 (elastic#68583)
  [Logs + Metrics UI] Clean up async plugin initialization (elastic#67654)
  APM Storybook fixes (elastic#68671)
  Upgrade EUI to v24.1.0 (elastic#68141)
  [ML] DF Analytics: Creation wizard part 2 (elastic#68462)
  [Uptime] Fix race on overview page query (elastic#67843)
  Prefer using npm_execpath when spawning Yarn (elastic#68673)
  [Security] [Cases] Attach timeline to existing case (elastic#68580)
  Use Search API in Vega (elastic#68257)
  [Component templates] Table view (elastic#68031)
  [Uptime] Added relative date info in cert status column (elastic#67612)
  [Endpoint] Re-enable Functional test case for Endpoint related pages (elastic#68445)
  run page_load_metrics tests in visual regresssion jobs (elastic#68570)
  Enable exhaustive-deps; correct any lint warnings (elastic#68453)
  ...
@yctercero
Copy link
Contributor Author

Thanks @peluja1012 and @spong for the in depth review. I added follow ups to your comments in this pr #68739

yctercero added a commit that referenced this pull request Jun 10, 2020
### Summary

- Adds missing unit tests for relevant files missing them
- Changes filter search to fire request on 'Enter'
- Breaks out the main ExceptionViewer component into smaller components to make more readable and better tested
- Updates utility bar to have the specific list description text next to it as proposed by @spong in #68294 (comment)
- Adds loading state any time async request occurs
- Now fetches list on list type toggle (if user selects to view either only detections or endpoint items), before was simply filtering already fetched items
yctercero added a commit to yctercero/kibana that referenced this pull request Jun 10, 2020
### Summary

- Adds missing unit tests for relevant files missing them
- Changes filter search to fire request on 'Enter'
- Breaks out the main ExceptionViewer component into smaller components to make more readable and better tested
- Updates utility bar to have the specific list description text next to it as proposed by @spong in elastic#68294 (comment)
- Adds loading state any time async request occurs
- Now fetches list on list type toggle (if user selects to view either only detections or endpoint items), before was simply filtering already fetched items
yctercero added a commit that referenced this pull request Jun 10, 2020
### Summary

- Adds missing unit tests for relevant files missing them
- Changes filter search to fire request on 'Enter'
- Breaks out the main ExceptionViewer component into smaller components to make more readable and better tested
- Updates utility bar to have the specific list description text next to it as proposed by @spong in #68294 (comment)
- Adds loading state any time async request occurs
- Now fetches list on list type toggle (if user selects to view either only detections or endpoint items), before was simply filtering already fetched items
@yctercero yctercero deleted the exceptions-viewer-2 branch October 14, 2020 12:01
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@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.

None yet

7 participants