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.

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

Merged
merged 11 commits into from
Feb 17, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export interface HostResultList {
query_strategy_version: MetadataQueryStrategyVersions;
/* policy IDs and versions */
policy_info?: HostInfo['policy_info'];
host_status?: HostStatus;
Copy link
Contributor

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?

Copy link
Contributor Author

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

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const initialEndpointListState: Immutable<EndpointState> = {
endpointsTotalError: undefined,
queryStrategyVersion: undefined,
policyVersionInfo: undefined,
hostStatus: undefined,
};

/* eslint-disable-next-line complexity */
Expand All @@ -58,6 +59,7 @@ export const endpointListReducer: ImmutableReducer<EndpointState, AppAction> = (
request_page_index: pageIndex,
query_strategy_version: queryStrategyVersion,
policy_info: policyVersionInfo,
host_status: hostStatus,
} = action.payload;
return {
...state,
Expand All @@ -67,6 +69,7 @@ export const endpointListReducer: ImmutableReducer<EndpointState, AppAction> = (
pageIndex,
queryStrategyVersion,
policyVersionInfo,
hostStatus,
loading: false,
error: undefined,
};
Expand Down Expand Up @@ -109,6 +112,7 @@ export const endpointListReducer: ImmutableReducer<EndpointState, AppAction> = (
...state,
details: action.payload.metadata,
policyVersionInfo: action.payload.policy_info,
hostStatus: action.payload.host_status,
detailsLoading: false,
detailsError: undefined,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
HostPolicyResponseConfiguration,
HostPolicyResponseActionStatus,
MetadataQueryStrategyVersions,
HostStatus,
} from '../../../../../common/endpoint/types';
import { EndpointState, EndpointIndexUIQueryParams } from '../types';
import { extractListPaginationParams } from '../../../common/routing';
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we don't have an UNKNOWN type? If we do, would it make more sense here to set it to UNKOWN?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
AppLocation,
PolicyData,
MetadataQueryStrategyVersions,
HostStatus,
} from '../../../../common/endpoint/types';
import { ServerApiError } from '../../../common/types';
import { GetPackagesResponse } from '../../../../../fleet/common';
Expand Down Expand Up @@ -79,6 +80,9 @@ export interface EndpointState {
queryStrategyVersion?: MetadataQueryStrategyVersions;
/** The policy IDs and revision number of the corresponding agent, and endpoint. May be more recent than what's running */
policyVersionInfo?: HostInfo['policy_info'];
/** The status of the host, which is mapped to the Elastic Agent status in Fleet
*/
hostStatus?: HostStatus;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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};
Expand All @@ -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,
Expand All @@ -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}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

So here we use Offline for antyhing that is not online, error or unenrolling

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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}>
Expand All @@ -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 */
Copy link
Contributor

Choose a reason for hiding this comment

The 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 @ts-ignore ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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>
),
},
];
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -36,6 +37,7 @@ import {
policyResponseLoading,
policyResponseTimestamp,
policyVersionInfo,
hostStatusInfo,
} from '../../store/selectors';
import { EndpointDetails } from './endpoint_details';
import { PolicyResponse } from './policy_response';
Expand All @@ -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);
Expand All @@ -83,7 +86,7 @@ export const EndpointDetailsFlyout = memo(() => {
onClose={handleFlyoutClose}
style={{ zIndex: 4001 }}
data-test-subj="endpointDetailsFlyout"
size="s"
size="m"
>
<EuiFlyoutHeader hasBorder>
{loading ? (
Expand Down Expand Up @@ -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>
</>
)}
Expand All @@ -125,6 +132,14 @@ export const EndpointDetailsFlyout = memo(() => {

EndpointDetailsFlyout.displayName = 'EndpointDetailsFlyout';

const PolicyResponseFlyout = styled.div`
Copy link
Contributor

Choose a reason for hiding this comment

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

😨
Just want to confirm that this deviation from Eui is absolutely needed.

cc:/ @bfishel

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 }) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -203,7 +221,7 @@ const PolicyResponseFlyoutPanel = memo<{
/>
)}
</EuiFlyoutBody>
</>
</PolicyResponseFlyout>
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
*/
const PolicyResponseConfigAccordion = styled(EuiAccordion)`
.euiAccordion__triggerWrapper {
padding: ${(props) => props.theme.eui.paddingSizes.s};
padding: ${(props) => props.theme.eui.paddingSizes.xs};
}

&.euiAccordion-isOpen {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ export const POLICY_STATUS_TO_HEALTH_COLOR = Object.freeze<
failure: 'danger',
});

export const POLICY_STATUS_TO_BADGE_COLOR = Object.freeze<
{ [key in keyof typeof HostPolicyResponseActionStatus]: string }
>({
success: 'secondary',
warning: 'warning',
failure: 'danger',
});

export const POLICY_STATUS_TO_TEXT = Object.freeze<
{ [key in keyof typeof HostPolicyResponseActionStatus]: string }
>({
Expand Down
Loading