[Security Solution][Endpoint] Show consistent Endpoint Agent Status across security solution by paul-tavares · Pull Request #154961 · elastic/kibana

paul-tavares · 2023-04-13T20:32:46Z

Summary

PR refactors the display of the Endpoint Host Agent status across the multiple places in Security solution so that it uses 1 common component. The Status of an Endpoint Host also displays the isolation state of the endpoint along with any other Actions that might be pending against it.
The refactor also address a prior issue where new added response actions were not accounted for when the component displays (on hover in a popover) the itemized count of pending response actions against the host. The new implementation will display the summary of all pending actions going forward as they are added without having to remember to update the Component.
The Endpoint host agent pending actions display was also adjusted to ensure that isolation state is primary shown when there are multiple pending actions, so that its always visible to the user the state of isolation (see GIF below)

As a result of the refactor, several redundant components were deleted.

Pages that display Endpoint Host Agent status, and thus impacted by these changes, are:

Endpoint List
Endpoint Details flyout
Responder
Alert Details
Host Details

Screen capture:

Different condition for displaying isolation state:

Endpoint List:

Endpoint Details:

Alert Details:

Host overview:

Timeline:

Case:

Checklist

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

…ossible actions

…omponents/endpoint/`

…gent status

…mponent (delete prior component)

…reated HostInfoInterface (not immutable)

…ew-response-actions-to-pending-actions

semd · 2023-04-17T16:48:48Z

+      hostInfo: endpointData,
      endpointPolicy: endpointData.metadata.Endpoint.policy.applied.name,
      policyStatus: endpointData.metadata.Endpoint.policy.applied.status,
      sensorVersion: endpointData.metadata.agent.version,


If we put everything into the hostInfo entry, why do we need to have the other 3? Looks like we send redundant data.

@semd - True. I was waiting for a response from the 'security-threat-hunting' on whether we needed to maintain backwards compatibility in this API. Looks like Pablo replied today and based on his response, I plan to remove the extra data here in a subsequent PR (don't want to increase the scope of this one)

…ew-response-actions-to-pending-actions

machadoum

The threat-hunting-explore team code looks good to me.

machadoum · 2023-04-18T10:19:49Z

-  metadata: HostMetadata;
+// Host Information as return by the Host Details API.
+// NOTE:  `HostInfo` type is the original and defined as Immutable. If needing to
+//        work with data that is not mutable, use `HostInfo` instead


This message is confusing. It sounds like you are telling me to use HostInfo instead of HostInfo. 🤔

yeah, this does not sound like a clear message. I'll change it in my next PR or this one if I end up making another change.
Thanks for the feedback.

michaelolo24

Nice work!

ashokaditya

I've looked at the code changes partially. Will give it another go a bit later. I tested it out locally with emulator data and I'm seeing some inconsistency with the isolated badge when isolated. It says isolating... and then shows nothing sometimes and switches to isolated. Also, when released it shows isolated even after it is released. Here are a few clips I managed to record while testing locally.

Not sure if you saw it too.

badge doesn't show up until flyout is closed.

doesn't show isolated badge after isolating (goes into a "release"d UI without a badge and then shows isolated)
(have to see the clip a little further after the actions fail)

still shows isolated badge after release

ashokaditya · 2023-04-18T12:44:27Z

+>({
+  isolate: 'isolate',
+  unisolate: 'release',
+  execute: 'execute',
+  'get-file': 'get-file',
+  'running-processes': 'processes',
+  'kill-process': 'kill-process',
+  'suspend-process': 'suspend-process',
+});


You can also use lodash zipObject here e.g. Object.freeze(zipObject(RESPONSE_ACTION_API_COMMANDS_NAMES, CONSOLE_RESPONSE_ACTION_COMMANDS)). We'd need to ensure that the values in those two consts (RESPONSE_ACTION_API_COMMANDS_NAMES and CONSOLE_RESPONSE_ACTION_COMMANDS) are in the same order if you decide to use zipObject. Perhaps with comments.

Had not known about that lodash function. I kind of prefer this here only because it's its clearer what the values are and I don['t have to worry about the ordering of items in the arrays.

ashokaditya · 2023-04-18T12:48:16Z


-export type HostInfo = Immutable<{
-  metadata: HostMetadata;
+// Host Information as return by the Host Details API.


nit: ...returned...

ashokaditya · 2023-04-18T13:45:02Z

    return merge(hostMetadataDoc, overrides);
  }

+  /** Generates the complete `HostInfo` as return by a call to the Endpoint host details api */


nit: ...as returned by...

ashokaditya · 2023-04-18T13:52:46Z

+  EuiTextColor,
+  EuiToolTip,
+} from '@elastic/eui';
+import styled from 'styled-components';


Don't have to change but eui also exports a euiStyled that you can use.
import { euiStyled } from '@kbn/kibana-react-plugin/common'; Then you can use it the same way as we use styled

euiStyled(EuiFlexGroup) or euiStyled.div etc.

ashokaditya · 2023-04-18T13:55:27Z

+import { getAgentStatusText } from '../agent_status_text';
+
+const TOOLTIP_CONTENT_STYLES: React.CSSProperties = Object.freeze({ width: 150 });
+const AUTO_REFRESH_INTERVAL = 10000;


This is same as the DEFAULT_POLL_INTERVAL. No?

ashokaditya · 2023-04-18T13:55:37Z

+import { getAgentStatusText } from '../agent_status_text';
+
+const TOOLTIP_CONTENT_STYLES: React.CSSProperties = Object.freeze({ width: 150 });
+const AUTO_REFRESH_INTERVAL = 10000;


This is same as the DEFAULT_POLL_INTERVAL. No?

Good point. I think that's a common const so I will use that instead

ashokaditya · 2023-04-18T13:57:53Z

+   * If set to `true` (Default), then the endpoint isolation state and response actions count
+   * will be kept up to date by querying the API periodically
+   */
+  autoFresh?: boolean;


Maybe name thisauotRefresh instead for naming consistency

ahhh. Crap. Yeah, I meant to rename that and forgot. Will do it

…ew-response-actions-to-pending-actions

paul-tavares · 2023-04-18T16:12:02Z

@ashokaditya ,

Thanks for the reviews.
I have pushed through another update that includes most of your comments.

Re:

badge doesn't show up until flyout is closed

This is a known problem. The list currently only refreshes the data when the details flyout is closed. I think we might have an issue tracking this.

re:

doesn't show isolated badge after isolating (goes into a "release"d UI without a badge and then shows isolated)

And

still shows isolated badge after release

I will take a closer look at these, but I'm guessing they are due to how the component is being used from the endpoint list/details. The isolation state is given through using the data the list/details retrieves from the API, while the actions are auto-retrieved from inside of the component. So the isolation state (which comes from the Endpoint Metadata) might be taking a bit longer to update because of that refresh interval on the list.

I'm also wondering if what you highlighted here also exists prior to my changes 🤔

paul-tavares · 2023-04-18T19:21:32Z

@ashokaditya ,
So I think the issue is that I moved the sync of pending actions to the Component itself, which means that the refresh of the metadata -vs- the refresh of the pending actions is not sync'd up, thus why you are seeing inconsistencies.
I'm also finding that for the Endpoint list, this new component is not as efficient because it makes an API call to the pending actions status API for each host, while on the Endpoint list it makes only 1 single call to the pending actions api requesting the status of all of the endpoints in the current page of data.

I'm going to see if I can still address this today, but if not, then I can put up a separate PR to only add the execute and get-file actions to the pending list 😞

…tatus component

…tStatus

…ew-response-actions-to-pending-actions

paul-tavares · 2023-04-18T20:14:08Z

Ok. I made some changes that I think will mitigate the issues you were seeing on the Endpoint list/details (FYI: can't wait until we rip out Redux stuff out this area of the code).

There are still issues with refreshing the list data while the flyout is opened, but at least now the refreshed data (including the pending actions) are passed directly to the EndpointAgentStatus which should keep things in sync.

Also - FYI: the emulator seems to have some issues with sending the metadata update after an isolate/release response action. I'm not sure what the issue is (maybe timestamps and transform dropping the document?), but I tested with a real endpoint and it seem ok.

kibana-ci · 2023-04-18T21:35:46Z

💚 Build Succeeded

Buildkite Build
Commit: 64012d4

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3826	3821	-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`	16.0MB	9.0MB	-6.9MB

Page load bundle

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

id	before	after	diff
`securitySolution`	58.3KB	58.3KB	+2.0B

Unknown metric groups

async chunk count

id	before	after	diff
`securitySolution`	42	41	-1

ESLint disabled line counts

id	before	after	diff
`securitySolution`	432	435	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	512	515	+3

History

💛 Build #120801 was flaky 75d263d
💛 Build #120486 was flaky ff88091
💔 Build #120465 failed 5fcf556
💚 Build #120253 succeeded 44b2250
💛 Build #120119 was flaky 4947767
💔 Build #120096 failed 16a20c8

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

cc @paul-tavares

ashokaditya

I tested it out a couple more times. I am kind of seeing similar behavior on the endpoint list as I'm seeing on the main branch. There are still sometimes when I see the Isolating badge disappearing for a couple of seconds before Isolated badge shows up.

~~I'm not seeing the pending actions badge on the Hosts page on auto-refresh.~~

on main

We talked offline. The changes look alright on the PR

ashokaditya · 2023-04-19T15:05:45Z

+   * If set to `true` (Default), then the endpoint status and isolation/action counts will
+   * be kept up to date by querying the API periodically
+   */
+  autoFresh?: boolean;


Maybe you missed this one. We can alsochange in a new PR.

FYI: Done in #155455

…se and remove redundant endpoint data (#156709) ## Summary - Removes several pieces of data from the Host Details search strategy response that is now available via the `hostInfo` property of the data returned. This PR is a follow up from #154961 (comment)

related to changes in elastic/pull/154961 where `endpointDetails.hostInfo` was added

paul-tavares added 12 commits April 13, 2023 11:37

Const of Response actions API names to console command names

b7807c5

refactor of EndpointAgentAndIsolationStatus component to handle all p…

152b14a

…ossible actions

moved EndpointAgentAndIsolationStatus component to `public/common/c…

b952023

…omponents/endpoint/`

Fix pending actions flashing

9e08ccc

Renamed EndpointAgentAndIsolationStatus to EndpointAgentStatus

8052443

Replace host agent status in Agent list and details with new common a…

beb8994

…gent status

Refactored Host Details to use new <EndpointAgentStatus>

65760ce

new <EndpointAgentStatusById/> component

c1c2d61

Updated timeline alert details to use <EndpointAgentStatusById/> co…

b13126d

…mponent (delete prior component)

Delete <AgentStatus> component

473c0f0

Delete <EndpointHostIsolationStatus> - no longer needed

80a4ec1

fix exports from endpoint_agent_status

e735c29

paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.8.0 labels Apr 13, 2023

paul-tavares self-assigned this Apr 13, 2023

paul-tavares added 14 commits April 13, 2023 17:33

Updated test plan

68398c5

Types: enhanced pending_actions to show known response actions ++ c…

adca9b3

…reated HostInfoInterface (not immutable)

Test setup + initial set of tests for EndpointAgentStatus

c4add35

adjust type for EndpointPendingAction

905a19a

Remainder tests for EndpointAgentStatus

ced8443

some adjustments to tests

6939a71

Updated i18n translations

ac5cc59

Added generateHostInfo() method to EndpointMetadataGenerator

3f3293c

Fix failing Host panel test

177ae8b

Fix failing test for host overview

eaa7ea8

Fix test for responder header host component

86e3bd4

Fix endpoint list test by using correct data-test-subj

0ac70c4

Fix types

3425169

Merge remote-tracking branch 'upstream/main' into task/olm-6308-add-n…

16a20c8

…ew-response-actions-to-pending-actions

semd reviewed Apr 17, 2023

View reviewed changes

paul-tavares added 3 commits April 17, 2023 15:51

adjust logic for display of isolation state

7fba9ae

Merge remote-tracking branch 'upstream/main' into task/olm-6308-add-n…

5fcf556

…ew-response-actions-to-pending-actions

fix responder test

ff88091

machadoum approved these changes Apr 18, 2023

View reviewed changes

paul-tavares requested review from ashokaditya and removed request for patrykkopycinski April 18, 2023 12:03

michaelolo24 approved these changes Apr 18, 2023

View reviewed changes

paul-tavares requested review from joeypoon and removed request for gergoabraham April 18, 2023 12:16

ashokaditya reviewed Apr 18, 2023

View reviewed changes

paul-tavares added 2 commits April 18, 2023 12:11

feedback from ash

becc156

Merge remote-tracking branch 'upstream/main' into task/olm-6308-add-n…

75d263d

…ew-response-actions-to-pending-actions

paul-tavares added 4 commits April 18, 2023 16:02

Add pendingActions as optional prop to EndpointAgentStatus

4800d1d

Changed Endpoint list to pass along pending actions to EndpointAgentS…

9d284bf

…tatus component

change Endpoint Details to pass along pending actions to EndpointAgen…

64e4549

…tStatus

Merge remote-tracking branch 'upstream/main' into task/olm-6308-add-n…

64012d4

…ew-response-actions-to-pending-actions

ashokaditya reviewed Apr 19, 2023

View reviewed changes

ashokaditya approved these changes Apr 19, 2023

View reviewed changes

paul-tavares merged commit e35a1d4 into elastic:main Apr 19, 2023

kibanamachine added the backport:skip This PR does not require backporting label Apr 19, 2023

paul-tavares deleted the task/olm-6308-add-new-response-actions-to-pending-actions branch April 19, 2023 15:40

paul-tavares mentioned this pull request May 4, 2023

[Security Solution][Hosts] Update the host search strategy API response and remove redundant endpoint data #156709

Merged

ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Jun 29, 2023

remove redundant hostDetails

a6afaa0

related to changes in elastic/pull/154961 where `endpointDetails.hostInfo` was added

Conversation

paul-tavares commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screen capture:

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machadoum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelolo24 left a comment

Choose a reason for hiding this comment

Uh oh!

ashokaditya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paul-tavares commented Apr 18, 2023

Uh oh!

paul-tavares commented Apr 18, 2023

Uh oh!

paul-tavares commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibana-ci commented Apr 18, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

ashokaditya left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

paul-tavares commented Apr 13, 2023 •

edited

Loading

paul-tavares commented Apr 18, 2023 •

edited

Loading

ashokaditya left a comment •

edited

Loading