maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Security Solution][Exceptions Builder] - Fixes operator selection bug (#71178)

Browse source 
### Summary

This PR fixes two bugs in the exceptions builder. The first was that it was not allowing you to select any of the "excluded" operators. The second was that it was not adding the "and" badge when it should on initial render. It also adds unit tests for the EntryItemComponent.
This commit is contained in:
Yara Tercero 2020-07-08 22:30:35 -04:00 committed by GitHub
parent 3863921616
commit 8ad5ecef03
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 477 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

438
x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx Normal file
View file

@ -0,0 +1,438 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { mount } from 'enzyme';
import React from 'react';
import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui';
import { EntryItemComponent } from './entry_item';
import {
isOperator,
isNotOperator,
isOneOfOperator,
isNotOneOfOperator,
isInListOperator,
isNotInListOperator,
existsOperator,
doesNotExistOperator,
} from '../../autocomplete/operators';
import {
fields,
getField,
} from '../../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks.ts';
import { getFoundListSchemaMock } from '../../../../../../lists/common/schemas/response/found_list_schema.mock';
import { getEmptyValue } from '../../empty_value';
// mock out lists hook
const mockStart = jest.fn();
const mockResult = getFoundListSchemaMock();
jest.mock('../../../../common/lib/kibana');
jest.mock('../../../../lists_plugin_deps', () => {
const originalModule = jest.requireActual('../../../../lists_plugin_deps');
return {
...originalModule,
useFindLists: () => ({
loading: false,
start: mockStart.mockReturnValue(mockResult),
result: mockResult,
error: undefined,
}),
};
});
describe('EntryItemComponent', () => {
test('it renders fields disabled if "isLoading" is "true"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: undefined, operator: isOperator, value: undefined }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={true}
onChange={jest.fn()}
/>
);
expect(
wrapper.find('[data-test-subj="exceptionBuilderEntryField"] input').props().disabled
).toBeTruthy();
expect(
wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"] input').props().disabled
).toBeTruthy();
expect(
wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"] input').props().disabled
).toBeTruthy();
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldFormRow"]')).toHaveLength(0);
});
test('it renders field labels if "showLabel" is "true"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: undefined, operator: isOperator, value: undefined }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={true}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldFormRow"]')).not.toEqual(0);
});
test('it renders field values correctly when operator is "isOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual('is');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').text()).toEqual(
'1234'
);
});
test('it renders field values correctly when operator is "isNotOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isNotOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'is not'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatch"]').text()).toEqual(
'1234'
);
});
test('it renders field values correctly when operator is "isOneOfOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isOneOfOperator, value: ['1234'] }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'is one of'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').text()).toEqual(
'1234'
);
});
test('it renders field values correctly when operator is "isNotOneOfOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isNotOneOfOperator, value: ['1234'] }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'is not one of'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').text()).toEqual(
'1234'
);
});
test('it renders field values correctly when operator is "isInListOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isInListOperator, value: 'some-list-id' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'is in list'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldList"]').text()).toEqual(
'some name'
);
});
test('it renders field values correctly when operator is "isNotInListOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isNotInListOperator, value: 'some-list-id' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'is not in list'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldList"]').text()).toEqual(
'some name'
);
});
test('it renders field values correctly when operator is "existsOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: existsOperator, value: undefined }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'exists'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldExists"]').text()).toEqual(
getEmptyValue()
);
expect(
wrapper.find('[data-test-subj="exceptionBuilderEntryFieldExists"] input').props().disabled
).toBeTruthy();
});
test('it renders field values correctly when operator is "doesNotExistOperator"', () => {
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: doesNotExistOperator, value: undefined }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={jest.fn()}
/>
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryField"]').text()).toEqual('ip');
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryOperator"]').text()).toEqual(
'does not exist'
);
expect(wrapper.find('[data-test-subj="exceptionBuilderEntryFieldExists"]').text()).toEqual(
getEmptyValue()
);
expect(
wrapper.find('[data-test-subj="exceptionBuilderEntryFieldExists"] input').props().disabled
).toBeTruthy();
});
test('it invokes "onChange" when new field is selected and resets operator and value fields', () => {
const mockOnChange = jest.fn();
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={mockOnChange}
/>
);
((wrapper.find(EuiComboBox).at(0).props() as unknown) as {
onChange: (a: EuiComboBoxOptionOption[]) => void;
}).onChange([{ label: 'machine.os' }]);
expect(mockOnChange).toHaveBeenCalledWith(
{ field: 'machine.os', operator: 'included', type: 'match', value: undefined },
0
);
});
test('it invokes "onChange" when new operator is selected and resets value field', () => {
const mockOnChange = jest.fn();
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={mockOnChange}
/>
);
((wrapper.find(EuiComboBox).at(1).props() as unknown) as {
onChange: (a: EuiComboBoxOptionOption[]) => void;
}).onChange([{ label: 'is not' }]);
expect(mockOnChange).toHaveBeenCalledWith(
{ field: 'ip', operator: 'excluded', type: 'match', value: '' },
0
);
});
test('it invokes "onChange" when new value field is entered for match operator', () => {
const mockOnChange = jest.fn();
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isNotOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={mockOnChange}
/>
);
((wrapper.find(EuiComboBox).at(2).props() as unknown) as {
onCreateOption: (a: string) => void;
}).onCreateOption('126.45.211.34');
expect(mockOnChange).toHaveBeenCalledWith(
{ field: 'ip', operator: 'excluded', type: 'match', value: '126.45.211.34' },
0
);
});
test('it invokes "onChange" when new value field is entered for match_any operator', () => {
const mockOnChange = jest.fn();
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isOneOfOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={mockOnChange}
/>
);
((wrapper.find(EuiComboBox).at(2).props() as unknown) as {
onCreateOption: (a: string) => void;
}).onCreateOption('126.45.211.34');
expect(mockOnChange).toHaveBeenCalledWith(
{ field: 'ip', operator: 'included', type: 'match_any', value: ['126.45.211.34'] },
0
);
});
test('it invokes "onChange" when new value field is entered for list operator', () => {
const mockOnChange = jest.fn();
const wrapper = mount(
<EntryItemComponent
entry={{ field: getField('ip'), operator: isNotInListOperator, value: '1234' }}
entryIndex={0}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
isLoading={false}
onChange={mockOnChange}
/>
);
((wrapper.find(EuiComboBox).at(2).props() as unknown) as {
onChange: (a: EuiComboBoxOptionOption[]) => void;
}).onChange([{ label: 'some name' }]);
expect(mockOnChange).toHaveBeenCalledWith(
{
field: 'ip',
operator: 'excluded',
type: 'list',
list: { id: 'some-list-id', type: 'ip' },
},
0
);
});
});

35
x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx
View file

@ -67,13 +67,13 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
{
field: entry.field != null ? entry.field.name : undefined,
type: OperatorTypeEnum.MATCH,
operator: isOperator.operator,
operator: entry.operator.operator,
value: newField,
},
entryIndex
);
},
[onChange, entryIndex, entry.field]
[onChange, entryIndex, entry.field, entry.operator.operator]
);
const handleFieldMatchAnyValueChange = useCallback(
@ -82,13 +82,13 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
{
field: entry.field != null ? entry.field.name : undefined,
type: OperatorTypeEnum.MATCH_ANY,
operator: isOperator.operator,
operator: entry.operator.operator,
value: newField,
},
entryIndex
);
},
[onChange, entryIndex, entry.field]
[onChange, entryIndex, entry.field, entry.operator.operator]
);
const handleFieldListValueChange = useCallback(
@ -97,13 +97,13 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
{
field: entry.field != null ? entry.field.name : undefined,
type: OperatorTypeEnum.LIST,
operator: isOperator.operator,
operator: entry.operator.operator,
list: { id: newField.id, type: newField.type },
},
entryIndex
);
},
[onChange, entryIndex, entry.field]
[onChange, entryIndex, entry.field, entry.operator.operator]
);
const renderFieldInput = (isFirst: boolean): JSX.Element => {
@ -114,9 +114,9 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
selectedField={entry.field}
isLoading={isLoading}
isClearable={false}
isDisabled={indexPattern == null}
isDisabled={isLoading}
onChange={handleFieldChange}
data-test-subj="filterFieldSuggestionList"
data-test-subj="exceptionBuilderEntryField"
/>
);
@ -137,11 +137,11 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
placeholder={i18n.EXCEPTION_OPERATOR_PLACEHOLDER}
selectedField={entry.field}
operator={entry.operator}
isDisabled={false}
isDisabled={isLoading}
isLoading={false}
isClearable={false}
onChange={handleOperatorChange}
data-test-subj="filterFieldSuggestionList"
data-test-subj="exceptionBuilderEntryOperator"
/>
);
@ -165,12 +165,12 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
placeholder={i18n.EXCEPTION_FIELD_VALUE_PLACEHOLDER}
selectedField={entry.field}
selectedValue={value}
isDisabled={false}
isDisabled={isLoading}
isLoading={isLoading}
isClearable={false}
indexPattern={indexPattern}
onChange={handleFieldMatchValueChange}
data-test-subj="filterFieldSuggestionList"
data-test-subj="exceptionBuilderEntryFieldMatch"
/>
);
case OperatorTypeEnum.MATCH_ANY:
@ -180,12 +180,12 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
placeholder={i18n.EXCEPTION_FIELD_VALUE_PLACEHOLDER}
selectedField={entry.field}
selectedValue={values}
isDisabled={false}
isDisabled={isLoading}
isLoading={isLoading}
isClearable={false}
indexPattern={indexPattern}
onChange={handleFieldMatchAnyValueChange}
data-test-subj="filterFieldSuggestionList"
data-test-subj="exceptionBuilderEntryFieldMatchAny"
/>
);
case OperatorTypeEnum.LIST:
@ -195,17 +195,18 @@ export const EntryItemComponent: React.FC<EntryItemProps> = ({
selectedField={entry.field}
placeholder={i18n.EXCEPTION_FIELD_LISTS_PLACEHOLDER}
selectedValue={id}
isLoading={false}
isDisabled={false}
isLoading={isLoading}
isDisabled={isLoading}
isClearable={false}
onChange={handleFieldListValueChange}
data-test-subj="exceptionBuilderEntryFieldList"
/>
);
case OperatorTypeEnum.EXISTS:
return (
<AutocompleteFieldExistsComponent
placeholder={getEmptyValue()}
data-test-subj="filterFieldSuggestionList"
data-test-subj="exceptionBuilderEntryFieldExists"
/>
);
default:

13
x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx
View file

@ -29,6 +29,7 @@ interface ExceptionListItemProps {
isLoading: boolean;
indexPattern: IIndexPattern;
andLogicIncluded: boolean;
onCheckAndLogic: (item: ExceptionsBuilderExceptionItem[]) => void;
onDeleteExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void;
onExceptionItemChange: (item: ExceptionsBuilderExceptionItem, index: number) => void;
}
@ -41,6 +42,7 @@ export const ExceptionListItemComponent = React.memo<ExceptionListItemProps>(
indexPattern,
isLoading,
andLogicIncluded,
onCheckAndLogic,
onDeleteExceptionItem,
onExceptionItemChange,
}) => {
@ -70,11 +72,12 @@ export const ExceptionListItemComponent = React.memo<ExceptionListItemProps>(
onDeleteExceptionItem(updatedExceptionItem, exceptionItemIndex);
};
const entries = useMemo(
(): FormattedBuilderEntry[] =>
indexPattern != null ? getFormattedBuilderEntries(indexPattern, exceptionItem.entries) : [],
[indexPattern, exceptionItem.entries]
);
const entries = useMemo((): FormattedBuilderEntry[] => {
onCheckAndLogic([exceptionItem]);
return indexPattern != null
? getFormattedBuilderEntries(indexPattern, exceptionItem.entries)
: [];
}, [indexPattern, exceptionItem, onCheckAndLogic]);
const andBadge = useMemo((): JSX.Element => {
const badge = <AndOrBadge includeAntennas type="and" />;

19
x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx
View file

@ -77,15 +77,21 @@ export const ExceptionBuilder = ({
indexPatternConfig ?? []
);
const handleCheckAndLogic = (items: ExceptionsBuilderExceptionItem[]): void => {
setAndLogicIncluded((includesAnd: boolean): boolean => {
if (includesAnd) {
return true;
} else {
return items.filter(({ entries }) => entries.length > 1).length > 0;
}
});
};
// Bubble up changes to parent
useEffect(() => {
onChange({ exceptionItems: filterExceptionItems(exceptions), exceptionsToDelete });
}, [onChange, exceptionsToDelete, exceptions]);
const checkAndLogic = (items: ExceptionsBuilderExceptionItem[]): void => {
setAndLogicIncluded(items.filter(({ entries }) => entries.length > 1).length > 0);
};
const handleDeleteExceptionItem = (
item: ExceptionsBuilderExceptionItem,
itemIndex: number
@ -100,7 +106,7 @@ export const ExceptionBuilder = ({
...existingExceptions.slice(0, itemIndex),
...existingExceptions.slice(itemIndex + 1),
];
checkAndLogic(updatedExceptions);
handleCheckAndLogic(updatedExceptions);
return updatedExceptions;
});
@ -118,7 +124,7 @@ export const ExceptionBuilder = ({
...exceptions.slice(index + 1),
];
checkAndLogic(updatedExceptions);
handleCheckAndLogic(updatedExceptions);
setExceptions(updatedExceptions);
};
@ -214,6 +220,7 @@ export const ExceptionBuilder = ({
isLoading={indexPatternLoading}
exceptionItemIndex={index}
andLogicIncluded={andLogicIncluded}
onCheckAndLogic={handleCheckAndLogic}
onDeleteExceptionItem={handleDeleteExceptionItem}
onExceptionItemChange={handleExceptionItemChange}
/>