-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution][Endpoint][Admin] Endpoint Details UX Enhancements #90870
Changes from 8 commits
b640a50
decfc2b
ce28b6b
1a75c0a
0cb37b0
48bf508
0dff7b7
27ef8e4
f570137
63beff2
99e5c77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import { | |
HostPolicyResponseConfiguration, | ||
HostPolicyResponseActionStatus, | ||
MetadataQueryStrategyVersions, | ||
HostStatus, | ||
} from '../../../../../common/endpoint/types'; | ||
import { EndpointState, EndpointIndexUIQueryParams } from '../types'; | ||
import { extractListPaginationParams } from '../../../common/routing'; | ||
|
@@ -224,6 +225,16 @@ export const showView: (state: EndpointState) => 'policy_response' | 'details' = | |
} | ||
); | ||
|
||
/** | ||
* Returns the Host Status which is connected the fleet agent | ||
*/ | ||
export const hostStatusInfo: (state: Immutable<EndpointState>) => HostStatus = createSelector( | ||
(state) => state.hostStatus, | ||
(hostStatus) => { | ||
return hostStatus ? hostStatus : HostStatus.ERROR; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume we don't have an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup there is no unknown type, do you think it's better to make another type, or just set it to a different type? |
||
} | ||
); | ||
|
||
/** | ||
* Returns the Policy Response overall status | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,23 +10,27 @@ import { | |
EuiDescriptionList, | ||
EuiHealth, | ||
EuiHorizontalRule, | ||
EuiLink, | ||
EuiListGroup, | ||
EuiListGroupItem, | ||
EuiIcon, | ||
EuiText, | ||
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiBadge, | ||
} from '@elastic/eui'; | ||
import React, { memo, useMemo } from 'react'; | ||
import { FormattedMessage } from '@kbn/i18n/react'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { isPolicyOutOfDate } from '../../utils'; | ||
import { HostInfo, HostMetadata } from '../../../../../../common/endpoint/types'; | ||
import { HostInfo, HostMetadata, HostStatus } from '../../../../../../common/endpoint/types'; | ||
import { useEndpointSelector, useAgentDetailsIngestUrl } from '../hooks'; | ||
import { useNavigateToAppEventHandler } from '../../../../../common/hooks/endpoint/use_navigate_to_app_event_handler'; | ||
import { policyResponseStatus, uiQueryParams } from '../../store/selectors'; | ||
import { POLICY_STATUS_TO_HEALTH_COLOR } from '../host_constants'; | ||
import { | ||
POLICY_STATUS_TO_HEALTH_COLOR, | ||
POLICY_STATUS_TO_BADGE_COLOR, | ||
HOST_STATUS_TO_HEALTH_COLOR, | ||
} from '../host_constants'; | ||
import { FormattedDateAndTime } from '../../../../../common/components/endpoint/formatted_date_time'; | ||
import { useNavigateByRouterEventHandler } from '../../../../../common/hooks/endpoint/use_navigate_by_router_event_handler'; | ||
import { LinkToApp } from '../../../../../common/components/endpoint/link_to_app'; | ||
|
@@ -48,6 +52,7 @@ const LinkToExternalApp = styled.div` | |
margin-top: ${(props) => props.theme.eui.ruleMargins.marginMedium}; | ||
.linkToAppIcon { | ||
margin-right: ${(props) => props.theme.eui.ruleMargins.marginXSmall}; | ||
vertical-align: top; | ||
} | ||
.linkToAppPopoutIcon { | ||
margin-left: ${(props) => props.theme.eui.ruleMargins.marginXSmall}; | ||
|
@@ -57,7 +62,15 @@ const LinkToExternalApp = styled.div` | |
const openReassignFlyoutSearch = '?openReassignFlyout=true'; | ||
|
||
export const EndpointDetails = memo( | ||
({ details, policyInfo }: { details: HostMetadata; policyInfo?: HostInfo['policy_info'] }) => { | ||
({ | ||
details, | ||
policyInfo, | ||
hostStatus, | ||
}: { | ||
details: HostMetadata; | ||
policyInfo?: HostInfo['policy_info']; | ||
hostStatus: HostStatus; | ||
}) => { | ||
const agentId = details.elastic.agent.id; | ||
const { | ||
url: agentDetailsUrl, | ||
|
@@ -78,14 +91,33 @@ export const EndpointDetails = memo( | |
}), | ||
description: details.host.os.full, | ||
}, | ||
{ | ||
title: i18n.translate('xpack.securitySolution.endpoint.details.agentStatus', { | ||
defaultMessage: 'Agent Status', | ||
}), | ||
description: ( | ||
<EuiHealth | ||
color={HOST_STATUS_TO_HEALTH_COLOR[hostStatus]} | ||
data-test-subj="agentStatusHealth" | ||
> | ||
<EuiText size="m"> | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.list.hostStatusValue" | ||
defaultMessage="{hostStatus, select, online {Online} error {Error} unenrolling {Unenrolling} other {Offline}}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So here we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup -- this is just following the pattern from the endpoint list, since it already shows Agent status. i'm not sure if we wanted to further differentiate things. |
||
values={{ hostStatus }} | ||
/> | ||
</EuiText> | ||
</EuiHealth> | ||
), | ||
}, | ||
{ | ||
title: i18n.translate('xpack.securitySolution.endpoint.details.lastSeen', { | ||
defaultMessage: 'Last Seen', | ||
}), | ||
description: <FormattedDateAndTime date={new Date(details['@timestamp'])} />, | ||
}, | ||
]; | ||
}, [details]); | ||
}, [details, hostStatus]); | ||
|
||
const [policyResponseUri, policyResponseRoutePath] = useMemo(() => { | ||
// eslint-disable-next-line @typescript-eslint/naming-convention | ||
|
@@ -135,13 +167,15 @@ export const EndpointDetails = memo( | |
defaultMessage: 'Integration Policy', | ||
}), | ||
description: ( | ||
<> | ||
<EndpointPolicyLink | ||
policyId={details.Endpoint.policy.applied.id} | ||
data-test-subj="policyDetailsValue" | ||
> | ||
{details.Endpoint.policy.applied.name} | ||
</EndpointPolicyLink> | ||
<EuiFlexGroup alignItems="center"> | ||
<EuiFlexItem grow={false}> | ||
<EndpointPolicyLink | ||
policyId={details.Endpoint.policy.applied.id} | ||
data-test-subj="policyDetailsValue" | ||
> | ||
{details.Endpoint.policy.applied.name} | ||
</EndpointPolicyLink> | ||
</EuiFlexItem> | ||
<EuiFlexGroup gutterSize="s" alignItems="baseline"> | ||
{details.Endpoint.policy.applied.endpoint_policy_version && ( | ||
<EuiFlexItem grow={false}> | ||
|
@@ -167,33 +201,34 @@ export const EndpointDetails = memo( | |
</EuiFlexItem> | ||
)} | ||
</EuiFlexGroup> | ||
</> | ||
</EuiFlexGroup> | ||
), | ||
}, | ||
{ | ||
title: i18n.translate('xpack.securitySolution.endpoint.details.policyStatus', { | ||
defaultMessage: 'Policy Response', | ||
}), | ||
description: ( | ||
<EuiHealth | ||
color={POLICY_STATUS_TO_HEALTH_COLOR[policyStatus] || 'subdued'} | ||
data-test-subj="policyStatusHealth" | ||
/** EuiBadge requires additional unnecessary props when the href prop is defined */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does it require? Is it possible to pass empty values so that we can avoid the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so there's actually a bug where euibadge uses the wrong type when both an onclick and an href are used, it's being tracked in this ticket: elastic/eui#4530 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks! |
||
// @ts-ignore | ||
<EuiBadge | ||
color={POLICY_STATUS_TO_BADGE_COLOR[policyStatus] || 'default'} | ||
data-test-subj="policyStatusValue" | ||
href={policyResponseUri} | ||
onClick={policyStatusClickHandler} | ||
onClickAriaLabel={i18n.translate( | ||
'xpack.securitySolution.endpoint.details.policyStatus', | ||
{ defaultMessage: 'Policy Response' } | ||
)} | ||
> | ||
{/* eslint-disable-next-line @elastic/eui/href-or-on-click */} | ||
<EuiLink | ||
data-test-subj="policyStatusValue" | ||
href={policyResponseUri} | ||
onClick={policyStatusClickHandler} | ||
> | ||
<EuiText size="m"> | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.details.policyStatusValue" | ||
defaultMessage="{policyStatus, select, success {Success} warning {Warning} failure {Failed} other {Unknown}}" | ||
values={{ policyStatus }} | ||
/> | ||
</EuiText> | ||
</EuiLink> | ||
</EuiHealth> | ||
<EuiText size="m"> | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.details.policyStatusValue" | ||
defaultMessage="{policyStatus, select, success {Success} warning {Warning} failure {Failed} other {Unknown}}" | ||
values={{ policyStatus }} | ||
/> | ||
</EuiText> | ||
</EuiBadge> | ||
), | ||
}, | ||
]; | ||
|
@@ -248,7 +283,7 @@ export const EndpointDetails = memo( | |
onClick={handleReassignEndpointsClick} | ||
data-test-subj="endpointDetailsLinkToIngest" | ||
> | ||
<EuiIcon type="savedObjectsApp" className="linkToAppIcon" /> | ||
<EuiIcon type="managementApp" className="linkToAppIcon" /> | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.details.linkToIngestTitle" | ||
defaultMessage="Reassign Policy" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import { | |
import { useHistory } from 'react-router-dom'; | ||
import { FormattedMessage } from '@kbn/i18n/react'; | ||
import { i18n } from '@kbn/i18n'; | ||
import styled from 'styled-components'; | ||
import { useToasts } from '../../../../../common/lib/kibana'; | ||
import { useEndpointSelector } from '../hooks'; | ||
import { urlFromQueryParams } from '../url_from_query_params'; | ||
|
@@ -36,6 +37,7 @@ import { | |
policyResponseLoading, | ||
policyResponseTimestamp, | ||
policyVersionInfo, | ||
hostStatusInfo, | ||
} from '../../store/selectors'; | ||
import { EndpointDetails } from './endpoint_details'; | ||
import { PolicyResponse } from './policy_response'; | ||
|
@@ -57,6 +59,7 @@ export const EndpointDetailsFlyout = memo(() => { | |
} = queryParams; | ||
const details = useEndpointSelector(detailsData); | ||
const policyInfo = useEndpointSelector(policyVersionInfo); | ||
const hostStatus = useEndpointSelector(hostStatusInfo); | ||
const loading = useEndpointSelector(detailsLoading); | ||
const error = useEndpointSelector(detailsError); | ||
const show = useEndpointSelector(showView); | ||
|
@@ -83,7 +86,7 @@ export const EndpointDetailsFlyout = memo(() => { | |
onClose={handleFlyoutClose} | ||
style={{ zIndex: 4001 }} | ||
data-test-subj="endpointDetailsFlyout" | ||
size="s" | ||
size="m" | ||
> | ||
<EuiFlyoutHeader hasBorder> | ||
{loading ? ( | ||
|
@@ -112,7 +115,11 @@ export const EndpointDetailsFlyout = memo(() => { | |
{show === 'details' && ( | ||
<> | ||
<EuiFlyoutBody data-test-subj="endpointDetailsFlyoutBody"> | ||
<EndpointDetails details={details} policyInfo={policyInfo} /> | ||
<EndpointDetails | ||
details={details} | ||
policyInfo={policyInfo} | ||
hostStatus={hostStatus} | ||
/> | ||
</EuiFlyoutBody> | ||
</> | ||
)} | ||
|
@@ -125,6 +132,14 @@ export const EndpointDetailsFlyout = memo(() => { | |
|
||
EndpointDetailsFlyout.displayName = 'EndpointDetailsFlyout'; | ||
|
||
const PolicyResponseFlyout = styled.div` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😨 cc:/ @bfishel There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a lot of space in between the back to endpoint details link and the beginning of the policy response, which is why the padding adjusted was added |
||
.endpointDetailsPolicyResponseFlyoutBody { | ||
.euiFlyoutBody__overflowContent { | ||
padding-top: 0; | ||
} | ||
} | ||
`; | ||
|
||
const PolicyResponseFlyoutPanel = memo<{ | ||
hostMeta: HostMetadata; | ||
}>(({ hostMeta }) => { | ||
|
@@ -165,12 +180,15 @@ const PolicyResponseFlyoutPanel = memo<{ | |
}, [backToDetailsClickHandler, detailsUri]); | ||
|
||
return ( | ||
<> | ||
<PolicyResponseFlyout> | ||
<FlyoutSubHeader | ||
backButton={backButtonProp} | ||
data-test-subj="endpointDetailsPolicyResponseFlyoutHeader" | ||
/> | ||
<EuiFlyoutBody data-test-subj="endpointDetailsPolicyResponseFlyoutBody"> | ||
<EuiFlyoutBody | ||
data-test-subj="endpointDetailsPolicyResponseFlyoutBody" | ||
className="endpointDetailsPolicyResponseFlyoutBody" | ||
> | ||
<EuiText data-test-subj="endpointDetailsPolicyResponseFlyoutTitle"> | ||
<h4> | ||
<FormattedMessage | ||
|
@@ -203,7 +221,7 @@ const PolicyResponseFlyoutPanel = memo<{ | |
/> | ||
)} | ||
</EuiFlyoutBody> | ||
</> | ||
</PolicyResponseFlyout> | ||
); | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious: was this missing? or was it introduced recently and part of an merge upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe it was in the hostInfo type (aka the type for the endpoint list) but not for the details