[CTI] Adds Threat Intel Tab to Alert Summary Flyout #97185

ecezalp · 2021-04-14T20:47:17Z

Summary

Updates Threat Summary & Threat Details features to match the updated designs.

no threat - summary

no threat - threat intel

threat - summary

threat - threat intel

Notes

Relates to elastic/security-team#961 - confidence field is passing through with no additional engineering as you can see from the last attached image above.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

ecezalp · 2021-04-14T20:48:03Z

@elasticmachine merge upstream

rylnd · 2021-04-14T22:33:32Z

RE labels: I think you want auto-backport instead of backport, and release_note-wise I would qualify this as either enhancement or maybe even skip, since #95604 was already a feature

ecezalp · 2021-04-15T13:58:57Z

latest changes

updated design
minor cleanups

rylnd

This is looking fantastic! We just need to add the first_seen sorting on details and fix up a few minor UI things.

With this refactor it definitely feels like we're doing too much client-side data manipulation (independent of the JSON.parse 😅 ); it's likely not a huge performance concern as it's one alert, but the code is pretty hard to follow right now. I left some thoughts/pointers, but as always I'm happy to either a) pair on it together or b) be told I'm wrong 😄

rylnd · 2021-04-15T00:03:51Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

-  return (
+  threatDetailsRows: ThreatDetailsRow[][];
+}> = ({ threatDetailsRows }) =>
+  !threatDetailsRows[0] || threatDetailsRows[0].length === 0 ? (


nit: I think isEmpty would work here:

Suggested change

!threatDetailsRows[0] || threatDetailsRows[0].length === 0 ? (

isEmpty(threatDetailsRows[0]) ? (

rylnd · 2021-04-15T20:47:18Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

+      </EuiTitle>
+      <StyledSpan>
+        {i18n.IF_CTI_NOT_ENABLED}
+        <a


Please use EuiLink here, that'll add the "external link" indicator that you see elsewhere.

rylnd · 2021-04-15T20:54:50Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

-  >
+}) => {
+  const tooltipChild = linkFields.some((field) => field === fieldName) ? (
+    <a href={value} target="_blank" rel="noreferrer">


Ditto here with EuiLink

rylnd · 2021-04-15T20:55:42Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

-import { getDataFromSourceHits } from '../../../../common/utils/field_formatters';
-import { INDICATOR_DESTINATION_PATH } from '../../../../common/constants';
+
+const linkFields = ['threat.indicator.event.url', 'threat.indicator.event.reference'];


More magic strings, we'll need to consolidate those once the row renderer is merged 👍

rylnd · 2021-04-15T20:56:27Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

+`;
+
+const StyledSpan = styled.span`
+  color: ${euiPaletteGray(5)[2]};


Is this going to work with both light and dark themes? We may want to pull a semantic color from the theme itself.

rylnd · 2021-04-15T21:41:35Z

x-pack/plugins/security_solution/public/common/components/event_details/summary_view.tsx

+  }
+`;
+
+const StyledDiv = styled.div`


nit:

Suggested change

const StyledDiv = styled.div`

const FlexDiv = styled.div`

rylnd · 2021-04-15T21:43:48Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

+  align-items: center;
+`;
+
+const StyledEuiIcon = styled(EuiIcon)`


I would try to give more semantic names to these styled components you're creating; a Styled prefix doesn't tell someone looking at this code why it has the given CSS.

rylnd · 2021-04-15T21:58:40Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_details_view.tsx

+      <StyledSpan>
+        {i18n.IF_CTI_NOT_ENABLED}
+        <a
+          href="https://www.elastic.co/guide/en/beats/filebeat/7.12/filebeat-module-threatintel.html"


Check out the docLinks service for a more robust way to build this URI. I think docLinks.links.filebeat.base might be it?

rylnd · 2021-04-15T22:26:49Z

x-pack/plugins/security_solution/public/common/hooks/use_threat_intel_tabs.ts

+  'last_seen',
+];
+
+export const useThreatIntelTabs = (


My first impression is that this hook is doing a lot 😅 , and it's only used in one place. The only state I see that makes it hook-worthy is the cached computations, is that correct?

If nothing else, we should try to decompose this to make it more reusable/recomposable if we do see this being used in the future. If we can abstract these data-manipulation pieces out into pure functions, then the hook is just a little logic and a few simple uses of useMemo.

However this shakes out, some kind of tests around this would be nice as documentation to further support/encourage reuse, as well.

rylnd · 2021-04-15T23:22:57Z

x-pack/plugins/security_solution/public/common/hooks/use_threat_intel_tabs.ts

+    if (selectedTabId !== EventsViewType.summaryView) return rows;
+    return threatData
+      .reduce((acc, items) => {
+        items.forEach(({ field, originalValue }) => {


This forEach within the reduce seems indicative of us doing something unnecessary; is it perhaps that we're having to undo part of what getDataFromSourceHits does above?

At a high level, this seems like what we need, data-manipulation-wise:

const summaryFields = ['threat.indicator.matched.atomic', 'etc.']; const indicatorJSON = data.find(field === 'threat.indicator'); const indicatorFields = data.filter(isIndicatorField); const threatSummaryRows = buildThreatSummaryRows(summaryFields, indicatorFieldsFields); const threatDetailsRows = buildThreatDetailsRows(indicatorJSON, ALL_INDICATOR_FIELDS).sort('first_seen', 'desc');

pgayvallet · 2021-04-19T18:04:46Z

Got a review request, but it seems that no changes are impacting core files. Do you want review on something specific, or were the changes triggering the review reverted?

ecezalp · 2021-04-19T18:15:17Z

Got a review request, but it seems that no changes are impacting core files. Do you want review on something specific, or were the changes triggering the review reverted?

hey @pgayvallet, I have removed the small change I had made, so there should be nothing here for you.

I do have a question for you though! So my change used to be the addition of a link to docLinks. This failed check_published_api_changes step of the build. In the logs, there were some instructions to fix the error

warn To accept these changes run `node scripts/check_published_api_changes.js --accept` and then:
      	 1. Commit the updated documentation and API review file '/dev/shm/workspace/parallel/12/kibana/src/core/public/public.api.md' 
      	 2. Describe the change in your PR including whether it's a major, minor or patch

so I was wondering - if I were to add a new link in the future, once I run the script mentioned above, what exactly the commit message would be? Would it just be a major / minor / patch according to the release I would be targeting (for instance, minor for 7.13 in this change?) or is there a separate versioning I am not aware of? Would be happy to know.

rylnd

LGTM! Excellent work.

rylnd · 2021-04-19T17:51:38Z

x-pack/plugins/security_solution/common/cti/constants.ts

@@ -19,10 +19,14 @@ export const INDICATOR_MATCHED_TYPE = `${INDICATOR_DESTINATION_PATH}.${MATCHED_T
 export const EVENT_DATASET = 'event.dataset';
 export const EVENT_REFERENCE = 'event.reference';
 export const PROVIDER = 'provider';
+export const FIRSTSEEN = 'first_seen';


nit:

Suggested change

export const FIRSTSEEN = 'first_seen';

export const FIRST_SEEN = 'first_seen';

Not worth waiting for another build, though.

_ seemed to indicate nesting level - leaving as is now, I can update in a later commit

rylnd · 2021-04-19T18:23:00Z

...ugins/security_solution/public/common/components/event_details/empty_threat_details_view.tsx

+`;
+
+const EmptyThreatDetailsViewComponent: React.FC<{}> = () => {
+  const threatIntelDocsUrl = `${


rylnd · 2021-04-19T18:29:07Z

x-pack/plugins/security_solution/public/common/components/event_details/threat_summary_view.tsx

+  eventId: string
+) => {
+  return data
+    .reduce((acc, { field, originalValue }) => {


nit: I think if you reoriented this as SORTED_THREAT_SUMMARY_FIELDS.map().filter() you wouldn't need the 'insert at index' logic below.

kibanamachine · 2021-04-19T20:58:39Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2217	2222	+5

Async chunks

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

id	before	after	diff
`securitySolution`	7.3MB	7.4MB	+38.5KB

History

💛 Build #120683 was flaky 0499e28
💔 Build #120658 failed 455243f
💚 Build #120558 succeeded 067f90a
💚 Build #120165 succeeded fcefc94
💚 Build #119989 succeeded f518062

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

cc @ecezalp

kibanamachine · 2021-04-19T21:06:43Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Co-authored-by: Ece Özalp <ozale272@newschool.edu>

ecezalp added 3 commits April 14, 2021 14:53

adds Threat Intel tab

dbca7d5

adds no enrichment view

1d087cb

fixes tests

2832e9e

ecezalp added v7.13.0 release_note:feature Makes this part of the condensed release notes 7.13 candidate Team: CTI labels Apr 14, 2021

ecezalp requested review from rylnd and a team April 14, 2021 20:47

ecezalp self-assigned this Apr 14, 2021

Merge branch 'master' into security-team-998

87b9c9f

ecezalp added the backport label Apr 14, 2021

tiny refactor

9d0a577

ecezalp changed the title ~~Security team 998~~ [CTI] Adds Threat Intel Tab to Alert Summary Flyout Apr 14, 2021

updates no enrichment found design

f518062

ecezalp added auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes and removed backport release_note:feature Makes this part of the condensed release notes labels Apr 15, 2021

fixes overflow

fcefc94

rylnd requested changes Apr 15, 2021

View reviewed changes

kibanamachine and others added 2 commits April 19, 2021 09:31

Merge branch 'master' into security-team-998

067f90a

adds sorting by first_seen, removes hook

e8be273

ecezalp requested a review from a team as a code owner April 19, 2021 17:10

adds type guards

455243f

ecezalp requested a review from rylnd April 19, 2021 17:20

reverts addition of docLink item

0499e28

rylnd approved these changes Apr 19, 2021

View reviewed changes

updates threat summary row logic

5716b58

ecezalp merged commit a254f0f into elastic:master Apr 19, 2021

kibanamachine mentioned this pull request Apr 19, 2021

[7.x] [CTI] Adds Threat Intel Tab to Alert Summary Flyout (#97185) #97528

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 19, 2021

[CTI] Adds Threat Intel Tab to Alert Summary Flyout (elastic#97185)

0b11770

kibanamachine added a commit that referenced this pull request Apr 19, 2021

[CTI] Adds Threat Intel Tab to Alert Summary Flyout (#97185) (#97528)

b46611e

Co-authored-by: Ece Özalp <ozale272@newschool.edu>

jmikell821 mentioned this pull request Apr 30, 2021

[DOCS] Update "Managing detection alerts" topic elastic/security-docs#664

Closed

nastasha-solomon mentioned this pull request May 3, 2021

[DOCS] Updating "Manage detection alerts" topic elastic/security-docs#666

Merged

elasticmachine unassigned ecezalp Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CTI] Adds Threat Intel Tab to Alert Summary Flyout #97185

[CTI] Adds Threat Intel Tab to Alert Summary Flyout #97185

ecezalp commented Apr 14, 2021 •

edited

Loading

ecezalp commented Apr 14, 2021

rylnd commented Apr 14, 2021

ecezalp commented Apr 15, 2021

rylnd left a comment

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

rylnd Apr 15, 2021

pgayvallet commented Apr 19, 2021

ecezalp commented Apr 19, 2021 •

edited

Loading

rylnd left a comment

rylnd Apr 19, 2021

ecezalp Apr 19, 2021

rylnd Apr 19, 2021

rylnd Apr 19, 2021

ecezalp Apr 19, 2021

kibanamachine commented Apr 19, 2021

kibanamachine commented Apr 19, 2021

	!threatDetailsRows[0] \|\| threatDetailsRows[0].length === 0 ? (
	isEmpty(threatDetailsRows[0]) ? (

	export const FIRSTSEEN = 'first_seen';
	export const FIRST_SEEN = 'first_seen';

[CTI] Adds Threat Intel Tab to Alert Summary Flyout #97185

[CTI] Adds Threat Intel Tab to Alert Summary Flyout #97185

Conversation

ecezalp commented Apr 14, 2021 • edited Loading

Summary

Notes

Checklist

For maintainers

ecezalp commented Apr 14, 2021

rylnd commented Apr 14, 2021

ecezalp commented Apr 15, 2021

rylnd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet commented Apr 19, 2021

ecezalp commented Apr 19, 2021 • edited Loading

rylnd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Apr 19, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Apr 19, 2021

💚 Backport successful

ecezalp commented Apr 14, 2021 •

edited

Loading

ecezalp commented Apr 19, 2021 •

edited

Loading