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] Bug fix for topN on draggables #70450

Merged
merged 14 commits into from
Jul 3, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ import { IdToDataProvider } from '../../store/drag_and_drop/model';
import { State } from '../../store/types';
import { DataProvider } from '../../../timelines/components/timeline/data_providers/data_provider';
import { reArrangeProviders } from '../../../timelines/components/timeline/data_providers/helpers';
import { ACTIVE_TIMELINE_REDUX_ID } from '../top_n';
import { ADDED_TO_TIMELINE_MESSAGE } from '../../hooks/translations';
import { useAddToTimelineSensor } from '../../hooks/use_add_to_timeline';
import { displaySuccessToast, useStateToaster } from '../toasters';

import { TimelineId } from '../../../../common/types/timeline';
import {
addFieldToTimelineColumns,
addProviderToTimeline,
Expand All @@ -35,7 +34,7 @@ import {
userIsReArrangingProviders,
} from './helpers';

// @ts-ignore
// @ts-expect-error
window['__react-beautiful-dnd-disable-dev-warnings'] = true;

interface Props {
Expand Down Expand Up @@ -67,7 +66,7 @@ const onDragEndHandler = ({
destination: result.destination,
dispatch,
source: result.source,
timelineId: ACTIVE_TIMELINE_REDUX_ID,
timelineId: TimelineId.active,
});
} else if (providerWasDroppedOnTimeline(result)) {
addProviderToTimeline({
Expand All @@ -76,7 +75,7 @@ const onDragEndHandler = ({
dispatch,
onAddedToTimeline,
result,
timelineId: ACTIVE_TIMELINE_REDUX_ID,
timelineId: TimelineId.active,
});
} else if (fieldWasDroppedOnTimelineColumns(result)) {
addFieldToTimelineColumns({
Expand Down Expand Up @@ -130,7 +129,6 @@ export const DragDropContextWrapperComponent = React.memo<Props & PropsFromRedux
[dataProviders, activeTimelineDataProviders, browserFields]
);
return (
// @ts-ignore
<DragDropContext onDragEnd={onDragEnd} onBeforeCapture={onBeforeCapture} sensors={sensors}>
{children}
</DragDropContext>
Expand All @@ -152,7 +150,7 @@ const emptyActiveTimelineDataProviders: DataProvider[] = []; // stable reference

const mapStateToProps = (state: State) => {
const activeTimelineDataProviders =
timelineSelectors.getTimelineByIdSelector()(state, ACTIVE_TIMELINE_REDUX_ID)?.dataProviders ??
timelineSelectors.getTimelineByIdSelector()(state, TimelineId.active)?.dataProviders ??
emptyActiveTimelineDataProviders;
const dataProviders = dragAndDropSelectors.dataProvidersSelector(state) ?? emptyDataProviders;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useCallback, useEffect, useMemo, useState } from 'react';
import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react';
import {
Draggable,
DraggableProvided,
Expand All @@ -22,7 +22,7 @@ import { DataProvider } from '../../../timelines/components/timeline/data_provid
import { TruncatableText } from '../truncatable_text';
import { WithHoverActions } from '../with_hover_actions';

import { DraggableWrapperHoverContent } from './draggable_wrapper_hover_content';
import { DraggableWrapperHoverContent, useGetTimelineId } from './draggable_wrapper_hover_content';
import { getDraggableId, getDroppableId } from './helpers';
import { ProviderContainer } from './provider_container';

Expand Down Expand Up @@ -101,11 +101,13 @@ export const getStyle = (

export const DraggableWrapper = React.memo<Props>(
({ dataProvider, onFilterAdded, render, truncate }) => {
const draggableRef = useRef<HTMLDivElement | null>(null);
const [showTopN, setShowTopN] = useState<boolean>(false);
const toggleTopN = useCallback(() => {
setShowTopN(!showTopN);
}, [setShowTopN, showTopN]);

const [goGetTimelineId, setGoGetTimelineId] = useState(false);
const timelineId = useGetTimelineId(draggableRef, goGetTimelineId);
const [providerRegistered, setProviderRegistered] = useState(false);

const dispatch = useDispatch();
Expand Down Expand Up @@ -135,8 +137,10 @@ export const DraggableWrapper = React.memo<Props>(
<DraggableWrapperHoverContent
draggableId={getDraggableId(dataProvider.id)}
field={dataProvider.queryMatch.field}
goGetTimelineId={setGoGetTimelineId}
onFilterAdded={onFilterAdded}
showTopN={showTopN}
timelineId={timelineId}
toggleTopN={toggleTopN}
value={
typeof dataProvider.queryMatch.value !== 'number'
Expand All @@ -145,7 +149,7 @@ export const DraggableWrapper = React.memo<Props>(
}
/>
),
[dataProvider, onFilterAdded, showTopN, toggleTopN]
[dataProvider, onFilterAdded, showTopN, timelineId, toggleTopN]
);

const renderContent = useCallback(
Expand Down Expand Up @@ -184,7 +188,10 @@ export const DraggableWrapper = React.memo<Props>(
<ProviderContainer
{...provided.draggableProps}
{...provided.dragHandleProps}
ref={provided.innerRef}
ref={(e: HTMLDivElement) => {
provided.innerRef(e);
draggableRef.current = e;
}}
data-test-subj="providerContainer"
isDragging={snapshot.isDragging}
registerProvider={registerProvider}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ jest.mock('../../../timelines/components/manage_timeline', () => {
return {
...original,
useManageTimeline: () => ({
getManageTimelineById: jest.fn().mockReturnValue({ indexToAdd: [] }),
getTimelineFilterManager: mockGetTimelineFilterManager,
isManagedTimeline: jest.fn().mockReturnValue(false),
}),
Expand All @@ -63,8 +64,10 @@ const timelineId = TimelineId.active;
const field = 'process.name';
const value = 'nice';
const toggleTopN = jest.fn();
const goGetTimelineId = jest.fn();
const defaultProps = {
field,
goGetTimelineId,
showTopN: false,
timelineId,
toggleTopN,
Expand Down Expand Up @@ -130,6 +133,18 @@ describe('DraggableWrapperHoverContent', () => {
wrapper.find(`[data-test-subj="filter-${hoverAction}-value"]`).first().exists()
).toBe(false);
});

test(`it should call goGetTimelineId when user is over the 'Filter ${hoverAction} value' button`, () => {
const wrapper = mount(
<TestProviders>
<DraggableWrapperHoverContent {...{ ...defaultProps }} />
</TestProviders>
);
const button = wrapper.find(`[data-test-subj="filter-${hoverAction}-value"]`).first();
button.simulate('mouseenter');
expect(goGetTimelineId).toHaveBeenCalledWith(true);
});

describe('when run in the context of a timeline', () => {
let wrapper: ReactWrapper;
let onFilterAdded: () => void;
Expand All @@ -151,6 +166,7 @@ describe('DraggableWrapperHoverContent', () => {
</TestProviders>
);
});

test('when clicked, it adds a filter to the timeline when running in the context of a timeline', () => {
wrapper.find(`[data-test-subj="filter-${hoverAction}-value"]`).first().simulate('click');
wrapper.update();
Expand Down Expand Up @@ -347,7 +363,7 @@ describe('DraggableWrapperHoverContent', () => {
const aggregatableStringField = 'cloud.account.id';
const draggableId = 'draggable.id';

[false, true].forEach((showTopN) => {
[true].forEach((showTopN) => {
XavierM marked this conversation as resolved.
Show resolved Hide resolved
[value, null].forEach((maybeValue) => {
[draggableId, undefined].forEach((maybeDraggableId) => {
const shouldRender = !showTopN && maybeValue != null && maybeDraggableId != null;
Expand Down Expand Up @@ -459,6 +475,23 @@ describe('DraggableWrapperHoverContent', () => {
expect(wrapper.find('[data-test-subj="show-top-field"]').first().exists()).toBe(false);
});

test(`it should invokes goGetTimelineId when user is over the 'Show top field' button`, () => {
const whitelistedField = 'signal.rule.name';
const wrapper = mount(
<TestProviders>
<DraggableWrapperHoverContent
{...{
...defaultProps,
field: whitelistedField,
}}
/>
</TestProviders>
);
const button = wrapper.find(`[data-test-subj="show-top-field"]`).first();
button.simulate('mouseenter');
expect(goGetTimelineId).toHaveBeenCalledWith(true);
});

test(`invokes the toggleTopN function when the 'Show top field' button is clicked`, async () => {
const whitelistedField = 'signal.rule.name';
const wrapper = mount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
import React, { useCallback, useMemo } from 'react';
import React, { useCallback, useMemo, useState, useEffect } from 'react';
import { DraggableId } from 'react-beautiful-dnd';

import { getAllFieldsByName, useWithSource } from '../../containers/source';
Expand All @@ -19,20 +19,23 @@ import { allowTopN } from './helpers';
import * as i18n from './translations';
import { useManageTimeline } from '../../../timelines/components/manage_timeline';
import { TimelineId } from '../../../../common/types/timeline';
import { SELECTOR_TIMELINE_BODY_CLASS_NAME } from '../../../timelines/components/timeline/styles';

interface Props {
draggableId?: DraggableId;
field: string;
goGetTimelineId?: (args: boolean) => void;
onFilterAdded?: () => void;
showTopN: boolean;
timelineId?: string;
timelineId?: string | null;
toggleTopN: () => void;
value?: string[] | string | null;
}

const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
draggableId,
field,
goGetTimelineId,
onFilterAdded,
showTopN,
timelineId,
Expand All @@ -48,11 +51,10 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({

const filterManager = useMemo(
() =>
timelineId === TimelineId.active ||
(draggableId != null && draggableId?.includes(TimelineId.active))
timelineId === TimelineId.active
? getTimelineFilterManager(TimelineId.active)
: filterManagerBackup,
[draggableId, timelineId, getTimelineFilterManager, filterManagerBackup]
[timelineId, getTimelineFilterManager, filterManagerBackup]
);

const filterForValue = useCallback(() => {
Expand Down Expand Up @@ -85,6 +87,12 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [field, value, filterManager, onFilterAdded]);

const handleGoGetTimelineId = useCallback(() => {
if (goGetTimelineId != null) {
goGetTimelineId(true);
}
}, [goGetTimelineId]);

const { browserFields } = useWithSource();

return (
Expand All @@ -97,6 +105,7 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
data-test-subj="filter-for-value"
iconType="magnifyWithPlus"
onClick={filterForValue}
onMouseEnter={handleGoGetTimelineId}
/>
</EuiToolTip>
)}
Expand All @@ -109,6 +118,7 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
data-test-subj="filter-out-value"
iconType="magnifyWithMinus"
onClick={filterOutValue}
onMouseEnter={handleGoGetTimelineId}
/>
</EuiToolTip>
)}
Expand Down Expand Up @@ -139,6 +149,7 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
data-test-subj="show-top-field"
iconType="visBarVertical"
onClick={toggleTopN}
onMouseEnter={handleGoGetTimelineId}
/>
</EuiToolTip>
)}
Expand All @@ -148,6 +159,7 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
browserFields={browserFields}
field={field}
onFilterAdded={onFilterAdded}
timelineId={timelineId ?? null}
toggleTopN={toggleTopN}
value={value}
/>
Expand All @@ -172,3 +184,30 @@ const DraggableWrapperHoverContentComponent: React.FC<Props> = ({
DraggableWrapperHoverContentComponent.displayName = 'DraggableWrapperHoverContentComponent';

export const DraggableWrapperHoverContent = React.memo(DraggableWrapperHoverContentComponent);

export const useGetTimelineId = function (
elem: React.MutableRefObject<Element | null>,
getTimelineId: boolean = false
) {
const [timelineId, setTimelineId] = useState<string | null>(null);

useEffect(() => {
let startElem: Element | (Node & ParentNode) | null = elem.current;
if (startElem != null && getTimelineId) {
for (; startElem && startElem !== document; startElem = startElem.parentNode) {
XavierM marked this conversation as resolved.
Show resolved Hide resolved
const myElem: Element = startElem as Element;
if (
myElem != null &&
myElem.classList != null &&
myElem.classList.contains(SELECTOR_TIMELINE_BODY_CLASS_NAME) &&
myElem.hasAttribute('data-timeline-id')
) {
setTimelineId(myElem.getAttribute('data-timeline-id'));
break;
}
}
}
}, [elem, getTimelineId]);

return timelineId;
};
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ export const getColumns = ({
data-test-subj="field-name"
fieldId={field}
onUpdateColumns={onUpdateColumns}
timelineId={contextId}
/>
</div>
)}
Expand Down
Loading