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][Exceptions] Prevents value list entries from co-existing with non value list entries #72995

Merged
merged 4 commits into from
Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,17 @@ export const EXCEPTION_OPERATORS: OperatorOption[] = [
isInListOperator,
isNotInListOperator,
];

export const EXCEPTION_OPERATORS_SANS_LISTS: OperatorOption[] = [
isOperator,
isNotOperator,
isOneOfOperator,
isNotOneOfOperator,
existsOperator,
doesNotExistOperator,
];

export const EXCEPTION_OPERATORS_ONLY_LISTS: OperatorOption[] = [
isInListOperator,
isNotInListOperator,
];
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ export const AddExceptionModal = memo(function AddExceptionModal({
signalIndexName,
]);

const isSubmitButtonDisabled = useCallback(
() => fetchOrCreateListError || exceptionItemsToAdd.length === 0,
const isSubmitButtonDisabled = useMemo(
() => fetchOrCreateListError || exceptionItemsToAdd.every((item) => item.entries.length === 0),
[fetchOrCreateListError, exceptionItemsToAdd]
);

Expand Down Expand Up @@ -362,7 +362,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
<EuiButton
onClick={onAddExceptionConfirm}
isLoading={addExceptionIsLoading}
isDisabled={isSubmitButtonDisabled()}
isDisabled={isSubmitButtonDisabled}
fill
>
{i18n.ADD_EXCEPTION}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ describe('BuilderEntryItem', () => {
title: 'logstash-*',
fields,
}}
showLabel={false}
showLabel={true}
listType="detection"
addNested={false}
onChange={jest.fn()}
Expand Down Expand Up @@ -245,7 +245,7 @@ describe('BuilderEntryItem', () => {
title: 'logstash-*',
fields,
}}
showLabel={false}
showLabel={true}
listType="detection"
addNested={false}
onChange={jest.fn()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
getEntryOnMatchAnyChange,
getEntryOnListChange,
} from './helpers';
import { EXCEPTION_OPERATORS_ONLY_LISTS } from '../../autocomplete/operators';

interface EntryItemProps {
entry: FormattedBuilderEntry;
Expand All @@ -35,6 +36,7 @@ interface EntryItemProps {
listType: ExceptionListType;
addNested: boolean;
onChange: (arg: BuilderEntry, i: number) => void;
onlyShowListOperators?: boolean;
}

export const BuilderEntryItem: React.FC<EntryItemProps> = ({
Expand All @@ -44,6 +46,7 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
addNested,
showLabel,
onChange,
onlyShowListOperators = false,
}): JSX.Element => {
const handleFieldChange = useCallback(
([newField]: IFieldType[]): void => {
Expand Down Expand Up @@ -124,11 +127,14 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
);

const renderOperatorInput = (isFirst: boolean): JSX.Element => {
const operatorOptions = getOperatorOptions(
entry,
listType,
entry.field != null && entry.field.type === 'boolean'
);
const operatorOptions = onlyShowListOperators
? EXCEPTION_OPERATORS_ONLY_LISTS
: getOperatorOptions(
entry,
listType,
entry.field != null && entry.field.type === 'boolean',
isFirst
);
const comboBox = (
<OperatorComponent
placeholder={i18n.EXCEPTION_OPERATOR_PLACEHOLDER}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface ExceptionListItemProps {
addNested: boolean;
onDeleteExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void;
onChangeExceptionItem: (item: ExceptionsBuilderExceptionItem, index: number) => void;
onlyShowListOperators?: boolean;
}

export const ExceptionListItemComponent = React.memo<ExceptionListItemProps>(
Expand All @@ -58,6 +59,7 @@ export const ExceptionListItemComponent = React.memo<ExceptionListItemProps>(
andLogicIncluded,
onDeleteExceptionItem,
onChangeExceptionItem,
onlyShowListOperators = false,
}) => {
const handleEntryChange = useCallback(
(entry: BuilderEntry, entryIndex: number): void => {
Expand Down Expand Up @@ -169,6 +171,7 @@ export const ExceptionListItemComponent = React.memo<ExceptionListItemProps>(
exceptionItemIndex === 0 && index === 0 && item.nested !== 'child'
}
onChange={handleEntryChange}
onlyShowListOperators={onlyShowListOperators}
/>
</EuiFlexItem>
{getDeleteButton(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,33 @@ import { getEntryExistsMock } from '../../../../../../lists/common/schemas/types
import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock';
import { getListResponseMock } from '../../../../../../lists/common/schemas/response/list_schema.mock';
import {
isOperator,
isOneOfOperator,
isNotOperator,
isNotOneOfOperator,
existsOperator,
doesNotExistOperator,
isInListOperator,
EXCEPTION_OPERATORS,
EXCEPTION_OPERATORS_SANS_LISTS,
existsOperator,
isInListOperator,
isNotOneOfOperator,
isNotOperator,
isOneOfOperator,
isOperator,
} from '../../autocomplete/operators';
import { FormattedBuilderEntry, BuilderEntry, ExceptionsBuilderExceptionItem } from '../types';
import { IIndexPattern, IFieldType } from '../../../../../../../../src/plugins/data/common';
import { EntryNested, Entry } from '../../../../lists_plugin_deps';
import { BuilderEntry, ExceptionsBuilderExceptionItem, FormattedBuilderEntry } from '../types';
import { IFieldType, IIndexPattern } from '../../../../../../../../src/plugins/data/common';
import { Entry, EntryNested } from '../../../../lists_plugin_deps';

import {
getFilteredIndexPatterns,
getFormattedBuilderEntry,
isEntryNested,
getFormattedBuilderEntries,
getUpdatedEntriesOnDelete,
getEntryFromOperator,
getOperatorOptions,
getEntryOnFieldChange,
getEntryOnOperatorChange,
getEntryOnMatchChange,
getEntryOnMatchAnyChange,
getEntryOnListChange,
getEntryOnMatchAnyChange,
getEntryOnMatchChange,
getEntryOnOperatorChange,
getFilteredIndexPatterns,
getFormattedBuilderEntries,
getFormattedBuilderEntry,
getOperatorOptions,
getUpdatedEntriesOnDelete,
isEntryNested,
} from './helpers';
import { OperatorOption } from '../../autocomplete/types';

Expand Down Expand Up @@ -672,6 +673,18 @@ describe('Exception builder helpers', () => {
const expected: OperatorOption[] = [isOperator, existsOperator];
expect(output).toEqual(expected);
});

test('it returns list operators if specified to', () => {
const payloadItem: FormattedBuilderEntry = getMockBuilderEntry();
const output = getOperatorOptions(payloadItem, 'detection', false, true);
expect(output).toEqual(EXCEPTION_OPERATORS);
});

test('it does not return list operators if specified not to', () => {
const payloadItem: FormattedBuilderEntry = getMockBuilderEntry();
const output = getOperatorOptions(payloadItem, 'detection', false, false);
expect(output).toEqual(EXCEPTION_OPERATORS_SANS_LISTS);
});
});

describe('#getEntryOnFieldChange', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
existsOperator,
isOneOfOperator,
EXCEPTION_OPERATORS,
EXCEPTION_OPERATORS_SANS_LISTS,
} from '../../autocomplete/operators';
import { OperatorOption } from '../../autocomplete/types';
import {
Expand All @@ -40,7 +41,6 @@ import { getEntryValue, getExceptionOperatorSelect } from '../helpers';
*
* @param patterns IIndexPattern containing available fields on rule index
* @param item exception item entry
* @param addNested boolean noting whether or not UI is currently
* set to add a nested field
*/
export const getFilteredIndexPatterns = (
Expand Down Expand Up @@ -295,12 +295,14 @@ export const getEntryFromOperator = (
*
* @param item
* @param listType
*
* @param isBoolean
* @param includeValueListOperators whether or not to include the 'is in list' and 'is not in list' operators
*/
export const getOperatorOptions = (
item: FormattedBuilderEntry,
listType: ExceptionListType,
isBoolean: boolean
isBoolean: boolean,
includeValueListOperators = true
): OperatorOption[] => {
if (item.nested === 'parent' || item.field == null) {
return [isOperator];
Expand All @@ -309,7 +311,11 @@ export const getOperatorOptions = (
} else if (item.nested != null && listType === 'detection') {
return isBoolean ? [isOperator, existsOperator] : [isOperator, isOneOfOperator, existsOperator];
} else {
return isBoolean ? [isOperator, existsOperator] : EXCEPTION_OPERATORS;
return isBoolean
? [isOperator, existsOperator]
: includeValueListOperators
? EXCEPTION_OPERATORS
: EXCEPTION_OPERATORS_SANS_LISTS;
}
};

Expand Down Expand Up @@ -547,3 +553,6 @@ export const getDefaultNestedEmptyEntry = (): EmptyNestedEntry => ({
type: OperatorTypeEnum.NESTED,
entries: [],
});

export const containsValueListEntry = (items: ExceptionsBuilderExceptionItem[]): boolean =>
items.some((item) => item.entries.some((entry) => entry.type === OperatorTypeEnum.LIST));
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import { BuilderButtonOptions } from './builder_button_options';
import { getNewExceptionItem, filterExceptionItems } from '../helpers';
import { ExceptionsBuilderExceptionItem, CreateExceptionListItemBuilderSchema } from '../types';
import { State, exceptionsBuilderReducer } from './reducer';
import { getDefaultEmptyEntry, getDefaultNestedEmptyEntry } from './helpers';
import {
containsValueListEntry,
getDefaultEmptyEntry,
getDefaultNestedEmptyEntry,
} from './helpers';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import exceptionableFields from '../exceptionable_fields.json';

Expand All @@ -44,6 +48,7 @@ const MyButtonsContainer = styled(EuiFlexItem)`

const initialState: State = {
disableAnd: false,
disableNested: false,
disableOr: false,
andLogicIncluded: false,
addNested: false,
Expand Down Expand Up @@ -82,12 +87,21 @@ export const ExceptionBuilder = ({
onChange,
}: ExceptionBuilderProps) => {
const [
{ exceptions, exceptionsToDelete, andLogicIncluded, disableAnd, disableOr, addNested },
{
exceptions,
exceptionsToDelete,
andLogicIncluded,
disableAnd,
disableNested,
disableOr,
addNested,
},
dispatch,
] = useReducer(exceptionsBuilderReducer(), {
...initialState,
disableAnd: isAndDisabled,
disableOr: isOrDisabled,
disableNested: isNestedDisabled,
});

const setUpdateExceptions = useCallback(
Expand Down Expand Up @@ -362,6 +376,7 @@ export const ExceptionBuilder = ({
isOnlyItem={exceptions.length === 1}
onDeleteExceptionItem={handleDeleteExceptionItem}
onChangeExceptionItem={handleExceptionItemChange}
onlyShowListOperators={containsValueListEntry(exceptions)}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand All @@ -379,7 +394,7 @@ export const ExceptionBuilder = ({
<BuilderButtonOptions
isOrDisabled={disableOr}
isAndDisabled={disableAnd}
isNestedDisabled={isNestedDisabled}
isNestedDisabled={disableNested}
isNested={addNested}
showNestedButton
onOrClicked={handleAddNewExceptionItem}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { ExceptionsBuilderExceptionItem } from '../types';
import { ExceptionListItemSchema } from '../../../../../public/lists_plugin_deps';
import { ExceptionListItemSchema, OperatorTypeEnum } from '../../../../../public/lists_plugin_deps';
import { getDefaultEmptyEntry } from './helpers';

export type ViewerModalName = 'addModal' | 'editModal' | null;

export interface State {
disableAnd: boolean;
disableNested: boolean;
disableOr: boolean;
andLogicIncluded: boolean;
addNested: boolean;
Expand Down Expand Up @@ -59,6 +60,9 @@ export const exceptionsBuilderReducer = () => (state: State, action: Action): St
const isAndDisabled =
lastEntry != null && lastEntry.type === 'nested' && lastEntry.entries.length === 0;
const isOrDisabled = lastEntry != null && lastEntry.type === 'nested';
const containsValueList = action.exceptions.some(
({ entries }) => entries.filter(({ type }) => type === OperatorTypeEnum.LIST).length > 0
);

return {
...state,
Expand All @@ -67,6 +71,7 @@ export const exceptionsBuilderReducer = () => (state: State, action: Action): St
addNested: isAddNested,
disableAnd: isAndDisabled,
disableOr: isOrDisabled,
disableNested: containsValueList,
};
}
case 'setDefault': {
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, { memo, useState, useCallback, useEffect } from 'react';
import React, { memo, useState, useCallback, useEffect, useMemo } from 'react';
import styled, { css } from 'styled-components';
import {
EuiModal,
Expand Down Expand Up @@ -146,6 +146,11 @@ export const EditExceptionModal = memo(function EditExceptionModal({
}
}, [shouldDisableBulkClose]);

const isSubmitButtonDisabled = useMemo(
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhh, nice 😉 I was gonna say maybe we need to return the errors from the builder, but really the only errors would be empty values or invalid values that would show with a red error state in the inputs.

() => exceptionItemsToAdd.every((item) => item.entries.length === 0),
[exceptionItemsToAdd]
);

const handleBuilderOnChange = useCallback(
({
exceptionItems,
Expand Down Expand Up @@ -261,7 +266,12 @@ export const EditExceptionModal = memo(function EditExceptionModal({
<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>

<EuiButton onClick={onEditExceptionConfirm} isLoading={addExceptionIsLoading} fill>
<EuiButton
onClick={onEditExceptionConfirm}
isLoading={addExceptionIsLoading}
isDisabled={isSubmitButtonDisabled}
fill
>
{i18n.EDIT_EXCEPTION_SAVE_BUTTON}
</EuiButton>
</EuiModalFooter>
Expand Down