Skip to content

Commit

Permalink
[Security Solution][Flyout] - fix analyzer preview loading and update…
Browse files Browse the repository at this point in the history
… hover actions in rule preview (elastic#175282)

## Summary

- Fixed a bug introduced by
elastic#174651: analyzer preview is stuck
in loading state because `_id` is not in the index for a preview alert.
Added back `kibana.alert.ancestor.id` when flyout is open in alert
preview.

- Refactor the use of security hover actions in flyout. The hover action
wrapper checks the type of document/scope (whether it is an alert, or in
a preview) to determine what actions to show on hover. Most hover
actions should behave consistently when flyout is in rule preview (do
not show filter options)
   - Related: elastic#173608 
- Not included in this pr: 1) hover actions in alert reason preview, 2)
hover actions in left panel entity details as the component is owned by
a different team and required greater refactor effort

- Fixed a UI bug on assignees breaking into multiple lines

![image](https://github.com/elastic/kibana/assets/18648970/96d909e3-b6bd-4a46-bc86-fbb473ce3b62)
 
### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
christineweng authored and fkanout committed Mar 4, 2024
1 parent 8908d1c commit 821f531
Show file tree
Hide file tree
Showing 22 changed files with 386 additions and 183 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { FC } from 'react';
import React, { useMemo } from 'react';
import { useLeftPanelContext } from '../context';
import { getSourcererScopeId } from '../../../../helpers';
import { useBasicDataFromDetailsData } from '../../../../timelines/components/side_panel/event_details/helpers';
import { SecurityCellActionType } from '../../../../actions/constants';
import {
CellActionsMode,
SecurityCellActions,
SecurityCellActionsTrigger,
} from '../../../../common/components/cell_actions';

interface CellActionsProps {
/**
* Field name
*/
field: string;
/**
* Field value
*/
value: string[] | string | null | undefined;
/**
* Boolean to indicate if value is an object array
*/
isObjectArray?: boolean;
/**
* React components to render
*/
children: React.ReactNode | string;
}

/**
* Security cell action wrapper for document details flyout
*/
export const CellActions: FC<CellActionsProps> = ({ field, value, isObjectArray, children }) => {
const { dataFormattedForFieldBrowser, scopeId, isPreview } = useLeftPanelContext();
const { isAlert } = useBasicDataFromDetailsData(dataFormattedForFieldBrowser);

const triggerId = isAlert
? SecurityCellActionsTrigger.DETAILS_FLYOUT
: SecurityCellActionsTrigger.DEFAULT;

const data = useMemo(() => ({ field, value }), [field, value]);
const metadata = useMemo(() => ({ scopeId, isObjectArray }), [scopeId, isObjectArray]);
const disabledActionTypes = useMemo(
() => (isPreview ? [SecurityCellActionType.FILTER, SecurityCellActionType.TOGGLE_COLUMN] : []),
[isPreview]
);

return (
<SecurityCellActions
data={data}
mode={CellActionsMode.HOVER_RIGHT}
triggerId={triggerId}
visibleCellActions={6}
sourcererScopeId={getSourcererScopeId(scopeId)}
metadata={metadata}
disabledActionTypes={disabledActionTypes}
>
{children}
</SecurityCellActions>
);
};

CellActions.displayName = 'CellActions';
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('CorrelationsDetails', () => {
it('renders all sections', () => {
jest
.mocked(useShowRelatedAlertsByAncestry)
.mockReturnValue({ show: true, indices: ['index1'] });
.mockReturnValue({ show: true, documentId: 'event-id', indices: ['index1'] });
jest
.mocked(useShowRelatedAlertsBySameSourceEvent)
.mockReturnValue({ show: true, originalEventId: 'originalEventId' });
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('CorrelationsDetails', () => {
it('should render no section and show error message if show values are false', () => {
jest
.mocked(useShowRelatedAlertsByAncestry)
.mockReturnValue({ show: false, indices: ['index1'] });
.mockReturnValue({ show: false, documentId: 'event-id', indices: ['index1'] });
jest
.mocked(useShowRelatedAlertsBySameSourceEvent)
.mockReturnValue({ show: false, originalEventId: 'originalEventId' });
Expand Down Expand Up @@ -144,7 +144,9 @@ describe('CorrelationsDetails', () => {
});

it('should render no section if values are null', () => {
jest.mocked(useShowRelatedAlertsByAncestry).mockReturnValue({ show: true });
jest
.mocked(useShowRelatedAlertsByAncestry)
.mockReturnValue({ show: true, documentId: 'event-id' });
jest.mocked(useShowRelatedAlertsBySameSourceEvent).mockReturnValue({ show: true });
jest.mocked(useShowRelatedAlertsBySession).mockReturnValue({ show: true });
jest.mocked(useShowRelatedCases).mockReturnValue(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,25 @@ export const CORRELATIONS_TAB_ID = 'correlations-details';
* Correlations displayed in the document details expandable flyout left section under the Insights tab
*/
export const CorrelationsDetails: React.FC = () => {
const { dataAsNestedObject, dataFormattedForFieldBrowser, eventId, getFieldsData, scopeId } =
useLeftPanelContext();
const {
dataAsNestedObject,
dataFormattedForFieldBrowser,
eventId,
getFieldsData,
scopeId,
isPreview,
} = useLeftPanelContext();

const { show: showAlertsByAncestry, indices } = useShowRelatedAlertsByAncestry({
const {
show: showAlertsByAncestry,
indices,
documentId,
} = useShowRelatedAlertsByAncestry({
getFieldsData,
dataAsNestedObject,
dataFormattedForFieldBrowser,
eventId,
isPreview,
});
const { show: showSameSourceAlerts, originalEventId } = useShowRelatedAlertsBySameSourceEvent({
getFieldsData,
Expand Down Expand Up @@ -82,9 +94,13 @@ export const CorrelationsDetails: React.FC = () => {
<RelatedAlertsBySession entityId={entityId} scopeId={scopeId} eventId={eventId} />
</EuiFlexItem>
)}
{showAlertsByAncestry && indices && (
{showAlertsByAncestry && documentId && indices && (
<EuiFlexItem>
<RelatedAlertsByAncestry indices={indices} scopeId={scopeId} documentId={eventId} />
<RelatedAlertsByAncestry
indices={indices}
scopeId={scopeId}
documentId={documentId}
/>
</EuiFlexItem>
)}
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React from 'react';
import { render } from '@testing-library/react';
import type { Anomalies } from '../../../../common/components/ml/types';
import { LeftPanelContext } from '../context';
import { TestProviders } from '../../../../common/mock';
import { HostDetails } from './host_details';
import { useMlCapabilities } from '../../../../common/components/ml/hooks/use_ml_capabilities';
Expand All @@ -22,6 +23,7 @@ import {
} from './test_ids';
import { EXPANDABLE_PANEL_CONTENT_TEST_ID } from '../../../shared/components/test_ids';
import { useRiskScore } from '../../../../entity_analytics/api/hooks/use_risk_score';
import { mockContextValue } from '../mocks/mock_context';

jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
Expand Down Expand Up @@ -122,10 +124,12 @@ const mockRelatedUsersResponse = {
loading: false,
};

const renderHostDetails = () =>
const renderHostDetails = (contextValue: LeftPanelContext) =>
render(
<TestProviders>
<HostDetails {...defaultProps} />
<LeftPanelContext.Provider value={contextValue}>
<HostDetails {...defaultProps} />
</LeftPanelContext.Provider>
</TestProviders>
);

Expand All @@ -139,13 +143,13 @@ describe('<HostDetails />', () => {
});

it('should render host details correctly', () => {
const { getByTestId } = renderHostDetails();
const { getByTestId } = renderHostDetails(mockContextValue);
expect(getByTestId(EXPANDABLE_PANEL_CONTENT_TEST_ID(HOST_DETAILS_TEST_ID))).toBeInTheDocument();
});

describe('Host overview', () => {
it('should render the HostOverview with correct dates and indices', () => {
const { getByTestId } = renderHostDetails();
const { getByTestId } = renderHostDetails(mockContextValue);
expect(mockUseHostDetails).toBeCalledWith({
id: 'entities-hosts-details-uuid',
startDate: from,
Expand All @@ -164,20 +168,20 @@ describe('<HostDetails />', () => {
});
mockUseRiskScore.mockReturnValue({ data: [], isAuthorized: true });

const { getByText } = renderHostDetails();
const { getByText } = renderHostDetails(mockContextValue);
expect(getByText('Host risk score')).toBeInTheDocument();
});

it('should not render host risk score when unauthorized', () => {
mockUseRiskScore.mockReturnValue({ data: [], isAuthorized: false });
const { queryByText } = renderHostDetails();
const { queryByText } = renderHostDetails(mockContextValue);
expect(queryByText('Host risk score')).not.toBeInTheDocument();
});
});

describe('Related users', () => {
it('should render the related user table with correct dates and indices', () => {
const { getByTestId } = renderHostDetails();
const { getByTestId } = renderHostDetails(mockContextValue);
expect(mockUseHostsRelatedUsers).toBeCalledWith({
from: timestamp,
hostName: 'test host',
Expand All @@ -194,7 +198,7 @@ describe('<HostDetails />', () => {
});
mockUseHasSecurityCapability.mockReturnValue(true);

const { queryAllByRole } = renderHostDetails();
const { queryAllByRole } = renderHostDetails(mockContextValue);
expect(queryAllByRole('columnheader').length).toBe(3);
expect(queryAllByRole('row')[1].textContent).toContain('test user');
expect(queryAllByRole('row')[1].textContent).toContain('100.XXX.XXX');
Expand All @@ -208,12 +212,12 @@ describe('<HostDetails />', () => {
});
mockUseHasSecurityCapability.mockReturnValue(false);

const { queryAllByRole } = renderHostDetails();
const { queryAllByRole } = renderHostDetails(mockContextValue);
expect(queryAllByRole('columnheader').length).toBe(2);
});

it('should not render host risk score column when license is not valid', () => {
const { queryAllByRole } = renderHostDetails();
const { queryAllByRole } = renderHostDetails(mockContextValue);
expect(queryAllByRole('columnheader').length).toBe(2);
});

Expand All @@ -224,7 +228,7 @@ describe('<HostDetails />', () => {
loading: false,
});

const { getByTestId } = renderHostDetails();
const { getByTestId } = renderHostDetails(mockContextValue);
expect(getByTestId(HOST_DETAILS_RELATED_USERS_TABLE_TEST_ID).textContent).toContain(
'No users identified'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
} from '@elastic/eui';
import type { EuiBasicTableColumn } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { getSourcererScopeId } from '../../../../helpers';
import { ExpandablePanel } from '../../../shared/components/expandable_panel';
import type { RelatedUser } from '../../../../../common/search_strategy/security_solution/related_entities/related_users';
import type { RiskSeverity } from '../../../../../common/search_strategy';
Expand All @@ -33,11 +32,7 @@ import { RiskScoreEntity } from '../../../../../common/search_strategy';
import { RiskScoreLevel } from '../../../../entity_analytics/components/severity/common';
import { DefaultFieldRenderer } from '../../../../timelines/components/field_renderers/field_renderers';
import { InputsModelId } from '../../../../common/store/inputs/constants';
import {
SecurityCellActions,
CellActionsMode,
SecurityCellActionsTrigger,
} from '../../../../common/components/cell_actions';
import { CellActions } from './cell_actions';
import { useGlobalTime } from '../../../../common/containers/use_global_time';
import { useSourcererDataView } from '../../../../common/containers/sourcerer';
import { manageQuery } from '../../../../common/components/page/manage_query';
Expand Down Expand Up @@ -135,20 +130,9 @@ export const HostDetails: React.FC<HostDetailsProps> = ({ hostName, timestamp, s
),
render: (user: string) => (
<EuiText grow={false} size="xs">
<SecurityCellActions
data={{
field: 'user.name',
value: user,
}}
mode={CellActionsMode.HOVER_RIGHT}
triggerId={SecurityCellActionsTrigger.DETAILS_FLYOUT}
visibleCellActions={6}
sourcererScopeId={getSourcererScopeId(scopeId)}
metadata={{ scopeId }}
showActionTooltips
>
<CellActions field={'user.name'} value={user}>
{user}
</SecurityCellActions>
</CellActions>
</EuiText>
),
},
Expand Down Expand Up @@ -190,7 +174,7 @@ export const HostDetails: React.FC<HostDetailsProps> = ({ hostName, timestamp, s
]
: []),
],
[isEntityAnalyticsAuthorized, scopeId]
[isEntityAnalyticsAuthorized]
);

const relatedUsersCount = useMemo(
Expand Down
Loading

0 comments on commit 821f531

Please sign in to comment.