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.

Sign up for GitHub

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] - Exceptions modal pt 2 #70886

Merged
merged 6 commits into from
Jul 10, 2020
Merged

[Security Solution][Exceptions] - Exceptions modal pt 2 #70886

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9816522
makes comment updates
dplumlee Jul 6, 2020
1459799
adds tests
dplumlee Jul 8, 2020
163701d
adds back non ecs data to timeline
dplumlee Jul 9, 2020
4262c59
comments
dplumlee Jul 9, 2020
b5c13d4
fixes jest tests
dplumlee Jul 9, 2020
0269167
fixes typo
dplumlee Jul 9, 2020
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
28 changes: 23 additions & 5 deletions ...ugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
defaultEndpointExceptionItems,
entryHasListType,
entryHasNonEcsType,
getMappedNonEcsValue,
} from '../helpers';
import { useFetchIndexPatterns } from '../../../../detections/containers/detection_engine/rules';

Expand All @@ -65,7 +66,7 @@ interface AddExceptionModalProps {
nonEcsData: TimelineNonEcsData[];
};
onCancel: () => void;
onConfirm: () => void;
onConfirm: (didCloseAlert?: boolean) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems from all the uses of onConfirm that the boolean argument is always present, do we need the ??

}

const Modal = styled(EuiModal)`
Expand Down Expand Up @@ -130,8 +131,8 @@ export const AddExceptionModal = memo(function AddExceptionModal({
);
const onSuccess = useCallback(() => {
displaySuccessToast(i18n.ADD_EXCEPTION_SUCCESS, dispatchToaster);
onConfirm();
}, [dispatchToaster, onConfirm]);
onConfirm(shouldCloseAlert);
}, [dispatchToaster, onConfirm, shouldCloseAlert]);

const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException(
{
Expand Down Expand Up @@ -193,6 +194,12 @@ export const AddExceptionModal = memo(function AddExceptionModal({
indexPatterns,
]);

useEffect(() => {
if (shouldDisableBulkClose === true) {
setShouldBulkCloseAlert(false);
}
}, [shouldDisableBulkClose]);

const onCommentChange = useCallback(
(value: string) => {
setComment(value);
Expand All @@ -214,18 +221,29 @@ export const AddExceptionModal = memo(function AddExceptionModal({
[setShouldBulkCloseAlert]
);

const retrieveAlertOsTypes = useCallback(() => {
const osTypes = getMappedNonEcsValue({
data: alertData.nonEcsData,
fieldName: 'host.os.family',
});
if (osTypes.length === 0) {
return ['windows', 'macos', 'linux'];
}
return osTypes;
}, [alertData]);

const enrichExceptionItems = useCallback(() => {
let enriched: Array<ExceptionListItemSchema | CreateExceptionListItemSchema> = [];
enriched =
comment !== ''
? enrichExceptionItemsWithComments(exceptionItemsToAdd, [{ comment }])
: exceptionItemsToAdd;
if (exceptionListType === 'endpoint') {
const osTypes = alertData ? ['windows'] : ['windows', 'macos', 'linux'];
const osTypes = alertData ? retrieveAlertOsTypes() : ['windows', 'macos', 'linux'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Might be able to just clean up a bit and add the alertData check within retrieveAlertOsTypes so if someone else later uses it they don't need to remember to make this check?

enriched = enrichExceptionItemsWithOS(enriched, osTypes);
}
return enriched;
}, [comment, exceptionItemsToAdd, exceptionListType, alertData]);
}, [comment, exceptionItemsToAdd, exceptionListType, alertData, retrieveAlertOsTypes]);

const onAddExceptionConfirm = useCallback(() => {
if (addOrUpdateExceptionItems !== null) {
Expand Down
12 changes: 10 additions & 2 deletions ...gins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { AddExceptionComments } from '../add_exception_comments';
import {
enrichExceptionItemsWithComments,
enrichExceptionItemsWithOS,
getOsTagValues,
getOperatingSystems,
entryHasListType,
entryHasNonEcsType,
} from '../helpers';
Expand Down Expand Up @@ -135,6 +135,12 @@ export const EditExceptionModal = memo(function EditExceptionModal({
indexPatterns,
]);

useEffect(() => {
if (shouldDisableBulkClose === true) {
setShouldBulkCloseAlert(false);
}
}, [shouldDisableBulkClose]);

const handleBuilderOnChange = useCallback(
({
exceptionItems,
Expand Down Expand Up @@ -167,7 +173,7 @@ export const EditExceptionModal = memo(function EditExceptionModal({
...(comment !== '' ? [{ comment }] : []),
]);
if (exceptionListType === 'endpoint') {
const osTypes = exceptionItem._tags ? getOsTagValues(exceptionItem._tags) : ['windows'];
const osTypes = exceptionItem._tags ? getOperatingSystems(exceptionItem._tags) : [];
enriched = enrichExceptionItemsWithOS(enriched, osTypes);
}
return enriched;
Expand Down Expand Up @@ -199,6 +205,8 @@ export const EditExceptionModal = memo(function EditExceptionModal({
{!isSignalIndexLoading && (
<>
<ModalBodySection className="builder-section">
<EuiText>{i18n.EXCEPTION_BUILDER_INFO}</EuiText>
<EuiSpacer />
<ExceptionBuilder
exceptionListItems={[exceptionItem]}
listType={exceptionListType}
Expand Down
2 changes: 1 addition & 1 deletion ...ecurity_solution/public/common/components/exceptions/edit_exception_modal/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const ENDPOINT_QUARANTINE_TEXT = i18n.translate(
);

export const EXCEPTION_BUILDER_INFO = i18n.translate(
'xpack.securitySolution.exceptions.addException.infoLabel',
'xpack.securitySolution.exceptions.editException.infoLabel',
{
defaultMessage: "Alerts are generated when the rule's conditions are met, except when:",
}
Expand Down
230 changes: 224 additions & 6 deletions x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ import {
getFormattedComments,
filterExceptionItems,
getNewExceptionItem,
formatOperatingSystems,
getEntryValue,
formatExceptionItemForUpdate,
enrichExceptionItemsWithComments,
enrichExceptionItemsWithOS,
entryHasListType,
entryHasNonEcsType,
} from './helpers';
import { FormattedEntry, DescriptionListItem, EmptyEntry } from './types';
import {
Expand All @@ -40,6 +47,9 @@ import {
getEntriesArrayMock,
} from '../../../../../lists/common/schemas/types/entries.mock';
import { getCommentsArrayMock } from '../../../../../lists/common/schemas/types/comments.mock';
import { ENTRIES } from '../../../../../lists/common/constants.mock';
import { ExceptionListItemSchema, EntriesArray } from '../../../../../lists/common/schemas';
import { IIndexPattern } from 'src/plugins/data/common';

describe('Exception helpers', () => {
beforeEach(() => {
Expand Down Expand Up @@ -248,6 +258,36 @@ describe('Exception helpers', () => {
});
});

describe('#getEntryValue', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding these!

it('returns "match" entry value', () => {
const payload = getEntryMatchMock();
const result = getEntryValue(payload);
const expected = 'some host name';
expect(result).toEqual(expected);
});

it('returns "match any" entry values', () => {
const payload = getEntryMatchAnyMock();
const result = getEntryValue(payload);
const expected = ['some host name'];
expect(result).toEqual(expected);
});

it('returns "exists" entry value', () => {
const payload = getEntryExistsMock();
const result = getEntryValue(payload);
const expected = undefined;
expect(result).toEqual(expected);
});

it('returns "list" entry value', () => {
const payload = getEntryListMock();
const result = getEntryValue(payload);
const expected = 'some-list-id';
expect(result).toEqual(expected);
});
});

describe('#formatEntry', () => {
test('it formats an entry', () => {
const payload = getEntryMatchMock();
Expand Down Expand Up @@ -278,27 +318,33 @@ describe('Exception helpers', () => {

describe('#getOperatingSystems', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be formatOperatingSystems?

test('it returns null if no operating system tag specified', () => {
const result = getOperatingSystems(['some tag', 'some other tag']);
const result = formatOperatingSystems(getOperatingSystems(['some tag', 'some other tag']));

expect(result).toEqual('');
});

test('it returns null if operating system tag malformed', () => {
const result = getOperatingSystems(['some tag', 'jibberos:mac,windows', 'some other tag']);
const result = formatOperatingSystems(
getOperatingSystems(['some tag', 'jibberos:mac,windows', 'some other tag'])
);

expect(result).toEqual('');
});

test('it returns formatted operating systems if space included in os tag', () => {
const result = getOperatingSystems(['some tag', 'os: mac', 'some other tag']);
const result = formatOperatingSystems(
getOperatingSystems(['some tag', 'os: macos', 'some other tag'])
);

expect(result).toEqual('Mac');
expect(result).toEqual('macOS');
});

test('it returns formatted operating systems if multiple os tags specified', () => {
const result = getOperatingSystems(['some tag', 'os: mac', 'some other tag', 'os:windows']);
const result = formatOperatingSystems(
getOperatingSystems(['some tag', 'os: macos', 'some other tag', 'os:windows'])
);

expect(result).toEqual('Mac, Windows');
expect(result).toEqual('macOS, Windows');
});
});

Expand Down Expand Up @@ -441,4 +487,176 @@ describe('Exception helpers', () => {
expect(exceptions).toEqual([{ ...rest, meta: undefined }]);
});
});

describe('#formatExceptionItemForUpdate', () => {
test('it should return correct update fields', () => {
const payload = getExceptionListItemSchemaMock();
const result = formatExceptionItemForUpdate(payload);
const expected = {
_tags: ['endpoint', 'process', 'malware', 'os:linux'],
comments: [],
description: 'This is a sample endpoint type exception',
entries: ENTRIES,
id: '1',
item_id: 'endpoint_list_item',
meta: {},
name: 'Sample Endpoint Exception List',
namespace_type: 'single',
tags: ['user added string for a tag', 'malware'],
type: 'simple',
};
expect(result).toEqual(expected);
});
});

describe('#enrichExceptionItemsWithComments', () => {
test('it should add comments to an exception item', () => {
const payload = [getExceptionListItemSchemaMock()];
const comments = getCommentsArrayMock();
const result = enrichExceptionItemsWithComments(payload, comments);
const expected = [
{
...getExceptionListItemSchemaMock(),
comments: getCommentsArrayMock(),
},
];
expect(result).toEqual(expected);
});

test('it should add comments to multiple exception items', () => {
const payload = [getExceptionListItemSchemaMock(), getExceptionListItemSchemaMock()];
const comments = getCommentsArrayMock();
const result = enrichExceptionItemsWithComments(payload, comments);
const expected = [
{
...getExceptionListItemSchemaMock(),
comments: getCommentsArrayMock(),
},
{
...getExceptionListItemSchemaMock(),
comments: getCommentsArrayMock(),
},
];
expect(result).toEqual(expected);
});
});

describe('#enrichExceptionItemsWithOS', () => {
test('it should add an os tag to an exception item', () => {
const payload = [getExceptionListItemSchemaMock()];
const osTypes = ['windows'];
const result = enrichExceptionItemsWithOS(payload, osTypes);
const expected = [
{
...getExceptionListItemSchemaMock(),
_tags: [...getExceptionListItemSchemaMock()._tags, 'os:windows'],
},
];
expect(result).toEqual(expected);
});

test('it should add multiple os tags to all exception items', () => {
const payload = [getExceptionListItemSchemaMock(), getExceptionListItemSchemaMock()];
const osTypes = ['windows', 'macos'];
const result = enrichExceptionItemsWithOS(payload, osTypes);
const expected = [
{
...getExceptionListItemSchemaMock(),
_tags: [...getExceptionListItemSchemaMock()._tags, 'os:windows', 'os:macos'],
},
{
...getExceptionListItemSchemaMock(),
_tags: [...getExceptionListItemSchemaMock()._tags, 'os:windows', 'os:macos'],
},
];
expect(result).toEqual(expected);
});

test('it should add os tag to all exception items without duplication', () => {
const payload = [
{ ...getExceptionListItemSchemaMock(), _tags: ['os:linux', 'os:windows'] },
{ ...getExceptionListItemSchemaMock(), _tags: ['os:linux'] },
];
const osTypes = ['windows'];
const result = enrichExceptionItemsWithOS(payload, osTypes);
const expected = [
{
...getExceptionListItemSchemaMock(),
_tags: ['os:linux', 'os:windows'],
},
{
...getExceptionListItemSchemaMock(),
_tags: ['os:linux', 'os:windows'],
},
];
expect(result).toEqual(expected);
});
});

describe('#entryHasListType', () => {
test('it should return false with an empty array', () => {
const payload: ExceptionListItemSchema[] = [];
const result = entryHasListType(payload);
expect(result).toEqual(false);
});

test("it should return false with exception items that don't contain a list type", () => {
const payload = [getExceptionListItemSchemaMock(), getExceptionListItemSchemaMock()];
const result = entryHasListType(payload);
expect(result).toEqual(false);
});

test('it should return true with exception items that do contain a list type', () => {
const payload = [
{
...getExceptionListItemSchemaMock(),
entries: [{ type: OperatorTypeEnum.LIST }] as EntriesArray,
},
getExceptionListItemSchemaMock(),
];
const result = entryHasListType(payload);
expect(result).toEqual(true);
});
});

describe('#entryHasNonEcsType', () => {
const mockEcsIndexPattern = {
title: 'testIndex',
fields: [
{
name: 'some.parentField',
},
{
name: 'some.not.nested.field',
},
{
name: 'nested.field',
},
],
} as IIndexPattern;

test('it should return false with an empty array', () => {
const payload: ExceptionListItemSchema[] = [];
const result = entryHasNonEcsType(payload, mockEcsIndexPattern);
expect(result).toEqual(false);
});

test("it should return false with exception items that don't contain a non ecs type", () => {
const payload = [getExceptionListItemSchemaMock(), getExceptionListItemSchemaMock()];
const result = entryHasNonEcsType(payload, mockEcsIndexPattern);
expect(result).toEqual(false);
});

test('it should return true with exception items that do contain a non ecs type', () => {
const payload = [
{
...getExceptionListItemSchemaMock(),
entries: [{ field: 'some.nonEcsField' }] as EntriesArray,
},
getExceptionListItemSchemaMock(),
];
const result = entryHasNonEcsType(payload, mockEcsIndexPattern);
expect(result).toEqual(true);
});
});
});
Loading