[RAC] [Security Solution] Hides the Show top <field> action in chart legends (#109566)

## Summary

Fixes <https://github.com/elastic/kibana/issues/108910> , which allowed users to launch a new `Top <field>` popover from an existing popover, to infinity (and beyond)

The `Show top <field>` action is now hidden in the `Top <field>` popover's chart legend, and also:

- In the legend items of charts throughout the Security Solution (e.g. on the `Overview` page, and in the `Trend` chart on the `Alerts` page)
- For items in the `Count` aggregation table on the `Alerts` page

## Screenshots

### Before (Top `signal.rule.name` popover)

![before-top-signal-rule-name](https://user-images.githubusercontent.com/4459398/130302784-00a6c24d-17c8-4361-979e-01b8467f100e.png)

_Above: It was possible to launch another `Top <field>` popover from the legend of an existing popover_

### After (Top `signal.rule.name` popover)

![after-top-signal-rule-name](https://user-images.githubusercontent.com/4459398/130302925-d5aaa1ff-9565-4374-aa87-bde5880cb64f.png)

_Above: It is no longer possible to launch another `Top <field>` popover from the legend of an existing popover_

### Before (Chart legends)

![before-overview](https://user-images.githubusercontent.com/4459398/130303169-dc6c6de3-a2ba-40fe-a1f0-fe0d78b9638c.png)

_Above: It was possible to launch a `Top <field>` popover from chart legends_

### After (Chart legends)

![after-overview](https://user-images.githubusercontent.com/4459398/130303519-2eb0a60e-c6cd-4659-b6b2-d5ba234f668f.png)

_Above: It is no longer possible to launch a `Top <field>` popover from chart legends_

### Before (`Count` items)

![before-count](https://user-images.githubusercontent.com/4459398/130304111-b37373cf-1afb-41b8-9f38-b5d9b37cdb2d.png)

_Above: It was possible to launch a `Top <field>` popover from `Count` items_

### After (`Count` items)

![after-count](https://user-images.githubusercontent.com/4459398/130304166-fb641fa2-b52e-44ff-8210-0e228a43330c.png)

_Above: It is no longer possible to launch a `Top <field>` popover from `Count` items_

cc @mdefazio
This commit is contained in:
Andrew Goldstein 2021-08-23 08:09:00 -06:00 committed by GitHub
parent 6e3af2b524
commit 6a61c43f06
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 78 additions and 99 deletions

View file

@ -54,4 +54,10 @@ describe('DraggableLegendItem', () => {
wrapper.find(`[data-test-subj="legend-item-${legendItem.dataProviderId}"]`).first().text()
).toEqual(legendItem.value);
});
it('always hides the Top N action for legend items', () => {
expect(
wrapper.find(`[data-test-subj="legend-item-${legendItem.dataProviderId}"]`).prop('hideTopN')
).toEqual(true);
});
});

View file

@ -36,6 +36,7 @@ const DraggableLegendItemComponent: React.FC<{
<DefaultDraggable
data-test-subj={`legend-item-${dataProviderId}`}
field={field}
hideTopN={true}
id={dataProviderId}
isDraggable={false}
timelineId={timelineId}

View file

@ -96,6 +96,7 @@ type RenderFunctionProp = (
interface Props {
dataProvider: DataProvider;
disabled?: boolean;
hideTopN?: boolean;
isDraggable?: boolean;
inline?: boolean;
render: RenderFunctionProp;
@ -125,6 +126,7 @@ export const getStyle = (
const DraggableOnWrapperComponent: React.FC<Props> = ({
dataProvider,
hideTopN = false,
onFilterAdded,
render,
timelineId,
@ -147,6 +149,7 @@ const DraggableOnWrapperComponent: React.FC<Props> = ({
showTopN,
} = useHoverActions({
dataProvider,
hideTopN,
onFilterAdded,
render,
timelineId,
@ -304,6 +307,7 @@ const DraggableOnWrapperComponent: React.FC<Props> = ({
const DraggableWrapperComponent: React.FC<Props> = ({
dataProvider,
hideTopN = false,
isDraggable = false,
onFilterAdded,
render,
@ -319,6 +323,7 @@ const DraggableWrapperComponent: React.FC<Props> = ({
showTopN,
} = useHoverActions({
dataProvider,
hideTopN,
isDraggable,
onFilterAdded,
render,
@ -363,6 +368,7 @@ const DraggableWrapperComponent: React.FC<Props> = ({
return (
<DraggableOnWrapperComponent
dataProvider={dataProvider}
hideTopN={hideTopN}
onFilterAdded={onFilterAdded}
render={render}
timelineId={timelineId}

View file

@ -655,6 +655,7 @@ describe('helpers', () => {
allowTopN({
browserField: aggregatableAllowedType,
fieldName: aggregatableAllowedType.name,
hideTopN: false,
})
).toBe(true);
});
@ -664,6 +665,7 @@ describe('helpers', () => {
allowTopN({
browserField: undefined,
fieldName: 'signal.rule.name',
hideTopN: false,
})
).toBe(true);
});
@ -678,6 +680,7 @@ describe('helpers', () => {
allowTopN({
browserField: nonAggregatableAllowedType,
fieldName: nonAggregatableAllowedType.name,
hideTopN: false,
})
).toBe(false);
});
@ -692,6 +695,7 @@ describe('helpers', () => {
allowTopN({
browserField: aggregatableNotAllowedType,
fieldName: aggregatableNotAllowedType.name,
hideTopN: false,
})
).toBe(false);
});
@ -703,6 +707,7 @@ describe('helpers', () => {
allowTopN({
browserField: missingAggregatable,
fieldName: missingAggregatable.name,
hideTopN: false,
})
).toBe(false);
});
@ -714,6 +719,7 @@ describe('helpers', () => {
allowTopN({
browserField: missingType,
fieldName: missingType.name,
hideTopN: false,
})
).toBe(false);
});
@ -723,6 +729,17 @@ describe('helpers', () => {
allowTopN({
browserField: undefined,
fieldName: 'non-allowlisted',
hideTopN: false,
})
).toBe(false);
});
test('it returns false when hideTopN is true', () => {
expect(
allowTopN({
browserField: aggregatableAllowedType,
fieldName: aggregatableAllowedType.name,
hideTopN: true, // <-- the Top N action shall not be shown for this (otherwise valid) field
})
).toBe(false);
});

View file

@ -92,9 +92,11 @@ export const addProviderToTimeline = ({
export const allowTopN = ({
browserField,
fieldName,
hideTopN,
}: {
browserField: Partial<BrowserField> | undefined;
fieldName: string;
hideTopN: boolean;
}): boolean => {
const isAggregatable = browserField?.aggregatable ?? false;
const fieldType = browserField?.type ?? '';
@ -181,5 +183,9 @@ export const allowTopN = ({
'signal.status',
].includes(fieldName);
if (hideTopN) {
return false;
}
return isAllowlistedNonBrowserField || (isAggregatable && isAllowedType);
};

View file

@ -36,6 +36,7 @@ exports[`draggables rendering it renders the default DefaultDraggable 1`] = `
},
}
}
hideTopN={false}
isDraggable={true}
render={[Function]}
/>

View file

@ -19,6 +19,7 @@ import {
import { Provider } from '../../../timelines/components/timeline/data_providers/provider';
export interface DefaultDraggableType {
hideTopN?: boolean;
id: string;
isDraggable?: boolean;
field: string;
@ -88,9 +89,11 @@ Content.displayName = 'Content';
* @param tooltipContent - defaults to displaying `field`, pass `null` to
* prevent a tooltip from being displayed, or pass arbitrary content
* @param queryValue - defaults to `value`, this query overrides the `queryMatch.value` used by the `DataProvider` that represents the data
* @param hideTopN - defaults to `false`, when true, the option to aggregate this field will be hidden
*/
export const DefaultDraggable = React.memo<DefaultDraggableType>(
({
hideTopN = false,
id,
isDraggable = true,
field,
@ -137,6 +140,7 @@ export const DefaultDraggable = React.memo<DefaultDraggableType>(
return (
<DraggableWrapper
dataProvider={dataProviderProp}
hideTopN={hideTopN}
isDraggable={isDraggable}
render={renderCallback}
timelineId={timelineId}

View file

@ -34,9 +34,10 @@ AdditionalContent.displayName = 'AdditionalContent';
const StyledHoverActionsContainer = styled.div<{
$showTopN: boolean;
$showOwnFocus: boolean;
$hideTopN: boolean;
$isActive: boolean;
}>`
min-width: 138px;
min-width: ${({ $hideTopN }) => `${$hideTopN ? '112px' : '138px'}`};
padding: ${(props) => `0 ${props.theme.eui.paddingSizes.s}`};
display: flex;
@ -91,6 +92,7 @@ interface Props {
enableOverflowButton?: boolean;
field: string;
goGetTimelineId?: (args: boolean) => void;
hideTopN?: boolean;
isObjectArray: boolean;
onFilterAdded?: () => void;
ownFocus: boolean;
@ -129,6 +131,7 @@ export const HoverActions: React.FC<Props> = React.memo(
field,
goGetTimelineId,
isObjectArray,
hideTopN = false,
onFilterAdded,
ownFocus,
showOwnFocus = true,
@ -207,6 +210,7 @@ export const HoverActions: React.FC<Props> = React.memo(
enableOverflowButton,
field,
handleHoverActionClicked,
hideTopN,
isObjectArray,
isOverflowPopoverOpen,
onFilterAdded,
@ -231,6 +235,7 @@ export const HoverActions: React.FC<Props> = React.memo(
onKeyDown={onKeyDown}
$showTopN={showTopN}
$showOwnFocus={showOwnFocus}
$hideTopN={hideTopN}
$isActive={isActive}
className={isActive ? 'hoverActions-active' : ''}
>

View file

@ -22,6 +22,7 @@ describe('useHoverActionItems', () => {
defaultFocusedButtonRef: null,
field: 'signal.rule.name',
handleHoverActionClicked: jest.fn(),
hideTopN: false,
isObjectArray: true,
ownFocus: false,
showTopN: false,
@ -112,4 +113,21 @@ describe('useHoverActionItems', () => {
);
});
});
test('it should hide the Top N action when hideTopN is true', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook(() => {
const testProps = {
...defaultProps,
hideTopN: true, // <-- hide the Top N action
};
return useHoverActionItems(testProps);
});
await waitForNextUpdate();
result.current.allActionItems.forEach((item) => {
expect(item.props['data-test-subj']).not.toEqual('hover-actions-show-top-n');
});
});
});
});

View file

@ -13,7 +13,7 @@ import { isEmpty } from 'lodash';
import { useKibana } from '../../lib/kibana';
import { getAllFieldsByName } from '../../containers/source';
import { allowTopN } from './utils';
import { allowTopN } from '../drag_and_drop/helpers';
import { useDeepEqualSelector } from '../../hooks/use_selector';
import { ColumnHeaderOptions, DataProvider, TimelineId } from '../../../../common/types/timeline';
import { SourcererScopeName } from '../../store/sourcerer/model';
@ -29,6 +29,7 @@ export interface UseHoverActionItemsProps {
enableOverflowButton?: boolean;
field: string;
handleHoverActionClicked: () => void;
hideTopN: boolean;
isObjectArray: boolean;
isOverflowPopoverOpen?: boolean;
itemsToShow?: number;
@ -56,6 +57,7 @@ export const useHoverActionItems = ({
enableOverflowButton,
field,
handleHoverActionClicked,
hideTopN,
isObjectArray,
isOverflowPopoverOpen,
itemsToShow = 2,
@ -182,6 +184,7 @@ export const useHoverActionItems = ({
allowTopN({
browserField: getAllFieldsByName(browserFields)[field],
fieldName: field,
hideTopN,
}) ? (
<ShowTopNButton
Component={enableOverflowButton ? EuiContextMenuItem : undefined}
@ -229,6 +232,7 @@ export const useHoverActionItems = ({
getFilterForValueButton,
getFilterOutValueButton,
handleHoverActionClicked,
hideTopN,
isObjectArray,
onFilterAdded,
ownFocus,

View file

@ -28,6 +28,7 @@ type RenderFunctionProp = (
interface Props {
dataProvider: DataProvider;
disabled?: boolean;
hideTopN: boolean;
isDraggable?: boolean;
inline?: boolean;
render: RenderFunctionProp;
@ -38,6 +39,7 @@ interface Props {
export const useHoverActions = ({
dataProvider,
hideTopN,
isDraggable,
onFilterAdded,
render,
@ -101,6 +103,7 @@ export const useHoverActions = ({
dataProvider={dataProvider}
draggableId={isDraggable ? getDraggableId(dataProvider.id) : undefined}
field={dataProvider.queryMatch.field}
hideTopN={hideTopN}
isObjectArray={false}
goGetTimelineId={setGoGetTimelineId}
onFilterAdded={onFilterAdded}
@ -120,6 +123,7 @@ export const useHoverActions = ({
closeTopN,
dataProvider,
handleClosePopOverTrigger,
hideTopN,
hoverActionsOwnFocus,
isDraggable,
onFilterAdded,

View file

@ -5,8 +5,6 @@
* 2.0.
*/
import { BrowserField } from '../../containers/source';
export const getAdditionalScreenReaderOnlyContext = ({
field,
value,
@ -20,98 +18,3 @@ export const getAdditionalScreenReaderOnlyContext = ({
return Array.isArray(value) ? `${field} ${value.join(' ')}` : `${field} ${value}`;
};
export const allowTopN = ({
browserField,
fieldName,
}: {
browserField: Partial<BrowserField> | undefined;
fieldName: string;
}): boolean => {
const isAggregatable = browserField?.aggregatable ?? false;
const fieldType = browserField?.type ?? '';
const isAllowedType = [
'boolean',
'geo-point',
'geo-shape',
'ip',
'keyword',
'number',
'numeric',
'string',
].includes(fieldType);
// TODO: remove this explicit allowlist when the ECS documentation includes alerts
const isAllowlistedNonBrowserField = [
'signal.ancestors.depth',
'signal.ancestors.id',
'signal.ancestors.rule',
'signal.ancestors.type',
'signal.original_event.action',
'signal.original_event.category',
'signal.original_event.code',
'signal.original_event.created',
'signal.original_event.dataset',
'signal.original_event.duration',
'signal.original_event.end',
'signal.original_event.hash',
'signal.original_event.id',
'signal.original_event.kind',
'signal.original_event.module',
'signal.original_event.original',
'signal.original_event.outcome',
'signal.original_event.provider',
'signal.original_event.risk_score',
'signal.original_event.risk_score_norm',
'signal.original_event.sequence',
'signal.original_event.severity',
'signal.original_event.start',
'signal.original_event.timezone',
'signal.original_event.type',
'signal.original_time',
'signal.parent.depth',
'signal.parent.id',
'signal.parent.index',
'signal.parent.rule',
'signal.parent.type',
'signal.rule.created_by',
'signal.rule.description',
'signal.rule.enabled',
'signal.rule.false_positives',
'signal.rule.filters',
'signal.rule.from',
'signal.rule.id',
'signal.rule.immutable',
'signal.rule.index',
'signal.rule.interval',
'signal.rule.language',
'signal.rule.max_signals',
'signal.rule.name',
'signal.rule.note',
'signal.rule.output_index',
'signal.rule.query',
'signal.rule.references',
'signal.rule.risk_score',
'signal.rule.rule_id',
'signal.rule.saved_id',
'signal.rule.severity',
'signal.rule.size',
'signal.rule.tags',
'signal.rule.threat',
'signal.rule.threat.tactic.id',
'signal.rule.threat.tactic.name',
'signal.rule.threat.tactic.reference',
'signal.rule.threat.technique.id',
'signal.rule.threat.technique.name',
'signal.rule.threat.technique.reference',
'signal.rule.timeline_id',
'signal.rule.timeline_title',
'signal.rule.to',
'signal.rule.type',
'signal.rule.updated_by',
'signal.rule.version',
'signal.status',
].includes(fieldName);
return isAllowlistedNonBrowserField || (isAggregatable && isAllowedType);
};

View file

@ -137,6 +137,7 @@ export const defaultCellActions: TGridCellAction[] = [
{allowTopN({
browserField: getAllFieldsByName(browserFields)[columnId],
fieldName: columnId,
hideTopN: false,
}) && (
<ShowTopNButton
Component={Component}

View file

@ -41,6 +41,7 @@ const getAlertsCountTableColumns = (
<DefaultDraggable
isDraggable={false}
field={selectedStackByOption}
hideTopN={true}
id={`alert-count-draggable-${selectedStackByOption}-${value}`}
value={value}
tooltipContent={null}

View file

@ -60,6 +60,7 @@ exports[`Field Renderers #hostIdRenderer it renders correctly against snapshot 1
},
}
}
hideTopN={false}
isDraggable={false}
render={[Function]}
/>
@ -82,6 +83,7 @@ exports[`Field Renderers #hostNameRenderer it renders correctly against snapshot
},
}
}
hideTopN={false}
isDraggable={false}
render={[Function]}
/>