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.

Sign up for GitHub

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

Jump to bottom

[Security Solution][Alert Flyout] Enable network preview and previews in table tab #190560

Merged
merged 4 commits into from
Sep 5, 2024
Merged

[Security Solution][Alert Flyout] Enable network preview and previews in table tab #190560

merged 4 commits into from
Sep 5, 2024

Conversation

christineweng
Copy link
Contributor

@christineweng christineweng commented Aug 14, 2024 •
edited
Loading

Summary

This PR is a refactor of preview links in document details flyout:

  1. Replace all the EuiLink component with a shared PreviewLink that renders a link based on field
  2. Added IP (network) flyout previews through out document details flyout
  3. Added preview capabilities for fields in table tab

Network previews

image

After

Screen.Recording.2024-08-14.at.3.31.36.PM.mov

Exceptions
IP addresses in entity details section are not yet worked on, as it is owned by the Explore team and therefore has impacts outside of alerts flyout.

image

How to test

  • Using the normal resolver script should generate host, user and ip data for testing.
  • To test ip's in highlighted fields, user should create a custom rule and add some ip fields to highlighted fields

Checklist

@christineweng christineweng added release_note:enhancement backport:skip This commit does not require backporting Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 labels Aug 14, 2024
@christineweng christineweng self-assigned this Aug 14, 2024
@christineweng
Copy link
Contributor Author

/ci

@christineweng christineweng marked this pull request as ready for review August 14, 2024 21:48
@christineweng christineweng requested review from a team as code owners August 14, 2024 21:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

This was referenced Aug 15, 2024
Closed
Closed
PhilippeOberti
PhilippeOberti reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

The code looks great and testing went well. I left a couple of minor comments.

Also, I feel like - while this might not have been the intent of this PR - it would be the right place to add the same preview logic to the rule preview inside the new Table tab. Unless I'm mistaken, we already have everything to display the rule preview. This would allow to handle this case

Screen.Recording.2024-08-19.at.1.28.55.PM.mov

which I think might be the last one for the table?

.../plugins/security_solution/public/flyout/document_details/shared/components/cell_actions.tsx Show resolved Hide resolved
.../plugins/security_solution/public/flyout/document_details/shared/components/preview_link.tsx Outdated Show resolved Hide resolved
.../plugins/security_solution/public/flyout/document_details/shared/components/preview_link.tsx Outdated Show resolved Hide resolved
@christineweng
Copy link
Contributor Author

The code looks great and testing went well. I left a couple of minor comments.

Also, I feel like - while this might not have been the intent of this PR - it would be the right place to add the same preview logic to the rule preview inside the new Table tab. Unless I'm mistaken, we already have everything to display the rule preview. This would allow to handle this case

Yes, good point. I wanted to confirm with product before doing this though. Alerts and entity flyout are currently enabled in alerts table, but the rule still goes to rule details page. That action may need to change to a flyout as well.

@christineweng christineweng force-pushed the flyout-preview-links branch 2 times, most recently from f93d8aa to ca259b2 Compare August 28, 2024 18:57
@christineweng
Copy link
Contributor Author

/ci

@christineweng christineweng mentioned this pull request Aug 29, 2024
Merged
2 tasks
PhilippeOberti
PhilippeOberti approved these changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a 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, thanks for making this great change!

@christineweng christineweng enabled auto-merge (squash) September 5, 2024 18:41
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 20.7MB 19.7MB -1.0MB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 86.5KB 86.5KB -19.0B
Unknown metric groups

async chunk count

id before after diff
securitySolution 97 95 -2

History

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

cc @christineweng

@christineweng christineweng merged commit 57ae12c into elastic:main Sep 5, 2024
39 checks passed
christineweng added a commit that referenced this pull request Sep 11, 2024
@christineweng
[Security Solution][Alert Flyout] Convert rule name to rule flyout an…
e01423d 
…d enable rule previews (#191764)

## Summary

This PR converts rule name in alert table to be a flyout (consistent
with host name and user name) and enables rule preview whenever rule
name is present.

This PR also moved the rule details component into its own
`rule_details` folder to be independent of the `document_details`
flyout.

Dependency: #190560 to be merged
first

New behavior:
- Rule link in alert table opens rule flyout
- Clicking the rule title goes to rule details page
- Clicking rule name in alert flyout opens rule preview



https://github.com/user-attachments/assets/857aa894-6253-4041-873a-18d6e8a003b6



### Checklist

- [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/main/packages/kbn-i18n/README.md)
- [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
gergoabraham pushed a commit to gergoabraham/kibana that referenced this pull request Sep 13, 2024
@christineweng @gergoabraham
[Security Solution][Alert Flyout] Convert rule name to rule flyout an…
de5cb6e 
…d enable rule previews (elastic#191764)

## Summary

This PR converts rule name in alert table to be a flyout (consistent
with host name and user name) and enables rule preview whenever rule
name is present.

This PR also moved the rule details component into its own
`rule_details` folder to be independent of the `document_details`
flyout.

Dependency: elastic#190560 to be merged
first

New behavior:
- Rule link in alert table opens rule flyout
- Clicking the rule title goes to rule details page
- Clicking rule name in alert flyout opens rule preview



https://github.com/user-attachments/assets/857aa894-6253-4041-873a-18d6e8a003b6



### Checklist

- [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/main/packages/kbn-i18n/README.md)
- [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
markov00 pushed a commit to markov00/kibana that referenced this pull request Sep 18, 2024
@christineweng @markov00
[Security Solution][Alert Flyout] Convert rule name to rule flyout an…
8689d36 
…d enable rule previews (elastic#191764)

## Summary

This PR converts rule name in alert table to be a flyout (consistent
with host name and user name) and enables rule preview whenever rule
name is present.

This PR also moved the rule details component into its own
`rule_details` folder to be independent of the `document_details`
flyout.

Dependency: elastic#190560 to be merged
first

New behavior:
- Rule link in alert table opens rule flyout
- Clicking the rule title goes to rule details page
- Clicking rule name in alert flyout opens rule preview



https://github.com/user-attachments/assets/857aa894-6253-4041-873a-18d6e8a003b6



### Checklist

- [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/main/packages/kbn-i18n/README.md)
- [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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhilippeOberti PhilippeOberti PhilippeOberti approved these changes

Assignees

@christineweng christineweng

Labels
backport:skip This commit does not require backporting release_note:enhancement Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@christineweng @elasticmachine @kibana-ci @PhilippeOberti @kibanamachine