[Security Solution] Use session view plugin to render session viewer in alerts, events and timeline #127520

kqualters-elastic · 2022-03-10T22:10:22Z

Summary

This pr adds the session view component to the actions component used by both data grid based tables in the alerts, hosts, network external events and rule based tables as well as the timeline flyout component. A new hook useSessionView contains most of the logic for handling timeline tabs, full screen functionality, etc, and existing patterns used for the tab.

Hosts page:

Sessions page:

Active timeline:

Active timeline fullscreen:

Alert flyout being opened from session view:

There is an active issue with the full screen behavior outside of the timeline flyout, we will need to figure out a way to not hardcode the height of the session view component to be 500px that it currently is, as it is not quite fullscreen.

Also added a feature flag in the kibana advanced settings, this setting defaults to true, and if turned off, makes it so that the button is no longer accessible in the leading control columns and can't be used to render the plugin, making it a feature flag. Easy to remove if we don't want to do that and just want the endpoint configuration to be the feature flag.

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
Unit or functional tests were updated or added to match the most common scenarios
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)

michaelolo24 · 2022-03-22T12:52:08Z

x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx

-  );
+  const graphOverlay = useMemo(() => {
+    const shouldShowOverlay =
+      (graphEventId != null && graphEventId.length > 0) || sessionViewId !== null;


do we need graphEventId.length > 0 ? Can we get away with shouldShowOverlay = graphEventId ?? sessionViewId?

Can we unify the != and !== approach in this statement just to use one? Could be graphEventId or sessionViewId as undefined?

I'm nervous to change it, for whatever reason, graph event id is set to empty string sometimes and is undefined at other times

michaelolo24 · 2022-03-22T12:55:15Z

x-pack/plugins/security_solution/public/timelines/components/graph_overlay/index.tsx

+    (state) => (getTimeline(state, timelineId) ?? timelineDefaults).sessionViewId
+  );
+  const sessionViewMain = useMemo(() => {
+    return sessionViewId !== null ? sessionView.getSessionView(sessionViewId) : null;


Any situation where it can be undefined? Would we want sessionViewId != null?

michaelolo24 · 2022-03-22T12:56:54Z

x-pack/plugins/security_solution/public/timelines/components/graph_overlay/index.tsx

@@ -219,7 +240,18 @@ const GraphOverlayComponent: React.FC<OwnProps> = ({ timelineId }) => {
    [defaultDataView.patternList, isInTimeline, timelinePatterns]
  );

-  if (fullScreen && !isInTimeline) {
+  if (!isInTimeline && sessionViewId !== null) {


We don't need the fullScreen check here anymore?

How it should behave in the fullScreen mode?

like analyzer i think, which it does, we just have a small issue with the hard coded unless specified height of 500px coming from here https://github.com/elastic/kibana/blob/main/x-pack/plugins/session_view/public/components/session_view/styles.ts#L16 @zizhouW @mitodrummer @opauloh I think we should change the styles here so that the search bar/detail panel button take up as much space as the constituent components need, and then the process tree grows to fill the remaining part of a top level container. Passing a height will be buggy/hard for no reason with the full screen functionality in data grid/timeline I think. What do y'all think?

so, the current way it works, it can't have auto height, because of infinite scrolling, so in order to enable full screen you would need to manually change the height you're passing to SessionView on FullScreen mode, something like this:

sessionView.getSessionView({ sessionEntityId, height: fullScreen ? 'calc(100vh - 118px)' : '500px' })

but having to know the height of the SessionView's search bar (118px), and the default height of the session view (500px) isn't ideal, so we are willing to change later to something more like this

sessionView.getSessionView({ sessionEntityId, isFullScreen: fullScreen })

michaelolo24 · 2022-03-22T12:57:51Z

x-pack/plugins/security_solution/public/timelines/components/timeline/body/actions/index.tsx

+    const { process } = ecsData;
+    const entryLeaderIds = process?.entry_leader?.entity_id;
+    if (entryLeaderIds !== undefined) {
+      return entryLeaderIds[0];


Always going to be the first one? And it's guaranteed to be an array of 1?

I'm not sure, but that's exactly why I wanted to get ecs changes in and then use real endpoint data to verify first. Seems to be the case so far.

We need to check array length at least.

michaelolo24 · 2022-03-22T12:59:38Z

x-pack/plugins/security_solution/public/timelines/components/timeline/body/actions/index.tsx

+    } else {
+      return null;
+    }
+  }, [ecsData]);


Don't expect ecsData to change, but any thoughts on this being ecsData.process?.entry_leader?.entity_id to be more specific? Not sure it would make a difference or if the linter would take it, but just a suggestion.

Agree, the better is to have a more specific to check.

michaelolo24 · 2022-03-22T13:01:22Z

x-pack/plugins/security_solution/public/timelines/components/timeline/body/actions/index.tsx

+                <EuiButtonIcon
+                  aria-label={i18n.VIEW_DETAILS_FOR_ROW({ ariaRowindex, columnValues })}
+                  data-test-subj="session-view-button"
+                  iconType="console"


Any word on when they expect the icon in the mocks to be added to eui?

the Icons are merged in Eui, and it seems we will have a quick release today 🤞 or at most tomorrow

pr here #128313

michaelolo24 · 2022-03-22T13:09:00Z

...plugins/security_solution/public/timelines/components/timeline/session_tab_content/index.tsx

+}
+
+const SessionTabContent: React.FC<Props> = ({ timelineId }) => {
+  const { sessionView } = useKibana().services;


I wonder if you can just have a useSessionView hook that internalizes all this logic and you can use that here and in the graph_overlay component https://github.com/elastic/kibana/pull/127520/files#diff-76ce75e94a9924487bf7b684f4a7404b250414469330abfbbf6ab0d34e770597, and you can just test that hook once.

michaelolo24 · 2022-03-22T13:10:16Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs_content/index.tsx

@@ -262,33 +273,36 @@ const TabsContentComponent: React.FC<BasicTimelineTab> = ({
    [appNotes, allTimelineNoteIds, timelineDescription]
  );

+  const setActiveTab = useCallback(


Thank you. ❤️

michaelolo24 · 2022-03-22T13:13:02Z

x-pack/plugins/security_solution/public/timelines/store/timeline/helpers.ts

@@ -287,6 +287,46 @@ export const updateGraphEventId = ({
  };
 };

+export const updateSessionViewEventId = ({


Will these two ever be different? Could updateSessionViewEventId and updateSessionViewSessionId be combined, or will they just be different in the future? It might be better to just leave them as one combined action to save a state update and then break them apart in the future if/when it's necessary

++ what @michaelolo24 suggests

this was placeholder stuff while I was testing if we could use entry_leader.entity_id in place of session_leader.entity_id etc, removing.

x-pack/plugins/session_view/public/components/process_tree/helpers.ts

michaelolo24 · 2022-03-22T13:15:29Z

x-pack/plugins/session_view/public/components/process_tree/hooks.ts

    const sessionIsInteractive = !!tty;

-    return sessionIsInteractive && parentIsASessionLeader && processIsAGroupLeader;
+    return !!(sessionIsInteractive && parentIsASessionLeader && processIsAGroupLeader);


Great catch

michaelolo24 · 2022-03-22T13:15:48Z

x-pack/plugins/session_view/public/components/process_tree/hooks.ts

@@ -170,15 +170,16 @@ export class ProcessImpl implements Process {
  // to be used as a source for the most up to date details
  // on the processes lifecycle.
  getDetailsMemo = memoizeOne((events: ProcessEvent[]) => {
+    // TODO: add these to generator


Can you make an issue and do it in a separate PR? Thanks!

YulNaumenko · 2022-03-23T05:01:43Z

x-pack/plugins/session_view/public/components/process_tree/hooks.ts

    const actionsToFind = [EventAction.fork, EventAction.exec, EventAction.end];
    const filtered = events.filter((processEvent) => {
-      return actionsToFind.includes(processEvent.event.action);
+      return true;


Why we still need that filter?

the this.events array contains all events for a particular process, which can include many event.action. e.g fork, exec, end, session_id_change, uid_change, etc... Not only that but any alert events will be stored in this.events as well (something we plan to break out into it's own this.alerts array). I assume this change was done while testing mock data perhaps? I believe @zizhouW will be making changes here regardless soon.

It might be worth updating process_events_route.ts to filter out the event.action types we don't need. like uid_change, gid_change, session_id_change etc... if we do that and store alerts in it's own array then we can do away with this filter.

ya exactly this change is not intended to be kept, was just for working with mock data

x-pack/plugins/timelines/common/ecs/process/index.ts

…dated

…kibana into safest-session-view-flyout

opauloh

LGTM!

michaelolo24 · 2022-03-29T18:36:31Z

x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx

-    [graphEventId, id]
-  );
+
+  const { DetailsPanel, SessionView, Navigation } = useSessionView({


This works, not sure if DetailsPanel and SessionView should be tied together like this, but I would have to think about it more...

kibana-ci · 2022-03-29T18:38:33Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3006	3009	+3

Async chunks

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

id	before	after	diff
`securitySolution`	4.8MB	4.8MB	+7.6KB
`sessionView`	51.6KB	51.8KB	+192.0B
total			+7.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`sessionView`	0	1	+1

Page load bundle

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

id	before	after	diff
`securitySolution`	250.1KB	250.4KB	+279.0B
`timelines`	286.3KB	286.4KB	+31.0B
total			+310.0B

Unknown metric groups

async chunk count

id	before	after	diff
`securitySolution`	23	24	+1

ESLint disabled in files

id	before	after	diff
`apm`	15	14	-1
`osquery`	5	4	-1
`securitySolution`	69	68	-1
`uptime`	7	6	-1
total			-4

ESLint disabled line counts

id	before	after	diff
`apm`	88	85	-3
`enterpriseSearch`	9	7	-2
`fleet`	47	46	-1
`osquery`	122	119	-3
`securitySolution`	440	441	+1
`uptime`	49	43	-6
total			-14

References to deprecated APIs

id	before	after	diff
`canvas`	70	64	-6
`dashboard`	78	72	-6
`data`	475	465	-10
`dataEnhanced`	55	49	-6
`discover`	26	20	-6
`fleet`	20	19	-1
`lens`	18	14	-4
`management`	2	1	-1
`maps`	456	330	-126
`monitoring`	40	28	-12
`upgradeAssistant`	12	7	-5
`visDefaultEditor`	205	155	-50
`visTypeVega`	4	3	-1
`visualizations`	17	13	-4
total			-238

Total ESLint disabled count

id	before	after	diff
`apm`	103	99	-4
`enterpriseSearch`	9	7	-2
`fleet`	55	54	-1
`osquery`	127	123	-4
`uptime`	56	49	-7
total			-18

History

💔 Build #34463 failed 5293a14
💔 Build #34420 failed 83502cd
💔 Build #34392 failed 545cddd
💔 Build #34338 failed 794efea
💔 Build #34310 failed e99a71b

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

michaelolo24 · 2022-03-29T19:26:01Z

x-pack/plugins/security_solution/public/timelines/components/graph_overlay/index.test.tsx

        <TestProviders>
-          <GraphOverlay timelineId={TimelineId.test} />
+          <GraphOverlay timelineId={TimelineId.test} SessionView={<div />} Navigation={<div />} />


I would set amockSessionView and mockNavigation so it's easier to change in the future

opauloh · 2022-03-29T19:46:13Z

x-pack/plugins/session_view/public/components/session_view/styles.ts

@@ -34,6 +34,11 @@ export const useStyles = ({ height = 500 }: StylesDeps) => {
      zIndex: 2,
    };

+    const nonGrowGroup: CSSObject = {


this can be removed now

@opauloh can you help us remove this in a follow up PR or coordinate with @kqualters-elastic on removing this? Thanks!

michaelolo24

Thanks for getting this done! Please let's create an issue with some bugs that we found regarding the event.process thing and please add beta tag in a follow up pr as well

YulNaumenko

LGTM

michaelolo24 · 2022-03-29T22:16:02Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs_content/index.tsx

@@ -140,7 +151,8 @@ const ActiveTimelineTab = memo<ActiveTimelineTabProps>(
    );

    const isGraphOrNotesTabs = useMemo(


We should probably make this more descriptive in a follow up since it's scope has expanded. Or add a comment to it's use case

michaelolo24 · 2022-03-29T22:18:55Z

...urity_solution/public/timelines/components/timeline/session_tab_content/use_session_view.tsx

+  activeTab: TimelineTabs;
+}
+
+const NavigationComponent: React.FC<NavigationProps> = ({


We should separate each of these into their own files when there's time and create test for each of them

michaelolo24 · 2022-03-29T22:19:00Z

...urity_solution/public/timelines/components/timeline/session_tab_content/use_session_view.tsx

+  timelineId,
+  timelineFullScreen,
+  toggleFullScreen,
+  graphEventId,


Not being used

michaelolo24 · 2022-03-29T22:19:17Z

...urity_solution/public/timelines/components/timeline/session_tab_content/use_session_view.tsx

+  timelineId: TimelineId;
+  timelineFullScreen: boolean;
+  toggleFullScreen: () => void;
+  graphEventId?: string;


kqualters-elastic added 2 commits March 10, 2022 12:06

Cherry-pick security/timelines changes

af077cd

Changes to make session_view work with generated data

0abc22b

kqualters-elastic added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.2.0 labels Mar 10, 2022

Demo tweaks

821c0c2

opauloh mentioned this pull request Mar 17, 2022

[Security solution][Session view] - Add Sessions tab into the Hosts page #127920

Merged

1 task

Merge branch 'main' into session-view-updated

21889d3

michaelolo24 reviewed Mar 22, 2022

View reviewed changes

x-pack/plugins/session_view/public/components/process_tree/helpers.ts Show resolved Hide resolved

michaelolo24 reviewed Mar 22, 2022

View reviewed changes

YulNaumenko self-requested a review March 23, 2022 00:46

YulNaumenko reviewed Mar 23, 2022

View reviewed changes

kqualters-elastic and others added 4 commits March 23, 2022 12:17

Cherry-pick security/timelines changes

86aa348

Changes to make session_view work with generated data

c3e7708

create detail panel hook for session view

d7cbe6a

add some tests

dd9452c

opauloh reviewed Mar 23, 2022

View reviewed changes

x-pack/plugins/timelines/common/ecs/process/index.ts Show resolved Hide resolved

kqualters-elastic and others added 3 commits March 24, 2022 11:20

Merge branch 'main' of github.com:elastic/kibana into session-view-up…

19ad8df

…dated

Changes to make session_view work with generated data

fa00f8b

Merge branch 'safest-session-view-flyout' of github.com:michaelolo24/…

b6d2b49

…kibana into safest-session-view-flyout

kqualters-elastic requested review from a team as code owners March 29, 2022 08:10

kqualters-elastic added 2 commits March 29, 2022 04:10

Merge remote-tracking branch 'upstream/main' into session-view-updated

794efea

Fix flyout usage with new hook

545cddd

michaelolo24 added the ci:deploy-cloud label Mar 29, 2022

kqualters-elastic and others added 5 commits March 29, 2022 07:52

Update failing snapshot

83502cd

Remove feature flag

678fdc6

Use correct sourcerer scope for panel

5293a14

Fix embarrassment

bf284c6

Merge branch 'main' into session-view-updated

bac80f5

michaelolo24 added ci:deploy-cloud and removed ci:deploy-cloud labels Mar 29, 2022

opauloh approved these changes Mar 29, 2022

View reviewed changes

michaelolo24 reviewed Mar 29, 2022

View reviewed changes

opauloh reviewed Mar 29, 2022

View reviewed changes

michaelolo24 approved these changes Mar 29, 2022

View reviewed changes

YulNaumenko approved these changes Mar 29, 2022

View reviewed changes

kqualters-elastic merged commit 33b85f8 into elastic:main Mar 29, 2022

michaelolo24 added release_note:feature Makes this part of the condensed release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Mar 29, 2022

michaelolo24 reviewed Mar 29, 2022

View reviewed changes

This was referenced Apr 5, 2022

[Security Solution][Investigations] - Add Session Viewer To Timeline #125948

Closed

[Security Solution][Investigations] - Add Session Viewer To Alert Table #127466

Closed

tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022

		@@ -140,7 +151,8 @@ const ActiveTimelineTab = memo<ActiveTimelineTabProps>(
		);

		const isGraphOrNotesTabs = useMemo(

[Security Solution] Use session view plugin to render session viewer in alerts, events and timeline #127520

[Security Solution] Use session view plugin to render session viewer in alerts, events and timeline #127520

Conversation

kqualters-elastic commented Mar 10, 2022 • edited Loading

Summary

Checklist

michaelolo24 Mar 22, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opauloh Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 Mar 22, 2022 • edited Loading

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

opauloh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Mar 29, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Public APIs missing exports

Page load bundle

async chunk count

ESLint disabled in files

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

YulNaumenko 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

kqualters-elastic commented Mar 10, 2022 •

edited

Loading

michaelolo24 Mar 22, 2022 •

edited

Loading

opauloh Mar 23, 2022 •

edited

Loading

michaelolo24 Mar 22, 2022 •

edited

Loading