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

[Security Solution][Detections] Handle conflicts on alert status update (#75492)

Browse source 
* Proceed on conflict when updating alert status

* Handle conflicts

* Don't let the user retry

* Tweak error messages

* Fix route

* Update add exception modal

* Reapply changes after fixing conflicts

* Type errors

* types

* Fix remaining conflicts

* Fix tests

* More test fixes

* Simplify onConflict evaluation

* Add callback return types

* Update translation paths

* Add missing import

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Madison Caldwell 2020-09-04 13:11:51 -04:00 committed by GitHub
parent c778646318
commit a7326e683a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 145 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts
View file

@ -283,6 +283,9 @@ export type Status = t.TypeOf<typeof status>;
export const job_status = t.keyof({ succeeded: null, failed: null, 'going to run': null });
export type JobStatus = t.TypeOf<typeof job_status>;
export const conflicts = t.keyof({ abort: null, proceed: null });
export type Conflicts = t.TypeOf<typeof conflicts>;
// TODO: Create a regular expression type or custom date math part type here
export const to = t.string;
export type To = t.TypeOf<typeof to>;

3
x-pack/plugins/security_solution/common/detection_engine/schemas/request/set_signal_status_schema.ts
View file

@ -6,13 +6,14 @@
import * as t from 'io-ts';
import { signal_ids, signal_status_query, status } from '../common/schemas';
import { conflicts, signal_ids, signal_status_query, status } from '../common/schemas';
export const setSignalsStatusSchema = t.intersection([
t.type({
status,
}),
t.partial({
conflicts,
signal_ids,
query: signal_status_query,
}),

50
x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx
View file

@ -26,6 +26,7 @@ import {
CreateExceptionListItemSchema,
ExceptionListType,
} from '../../../../../public/lists_plugin_deps';
import * as i18nCommon from '../../../translations';
import * as i18n from './translations';
import * as sharedI18n from '../translations';
import { TimelineNonEcsData, Ecs } from '../../../../graphql/types';
@ -49,6 +50,7 @@ import {
} from '../helpers';
import { ErrorInfo, ErrorCallout } from '../error_callout';
import { useFetchIndexPatterns } from '../../../../detections/containers/detection_engine/rules';
import { ExceptionsBuilderExceptionItem } from '../types';
export interface AddExceptionModalBaseProps {
ruleName: string;
@ -117,7 +119,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
>([]);
const [fetchOrCreateListError, setFetchOrCreateListError] = useState<ErrorInfo | null>(null);
const { addError, addSuccess } = useAppToasts();
const { addError, addSuccess, addWarning } = useAppToasts();
const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex();
const [
{ isLoading: isSignalIndexPatternLoading, indexPatterns: signalIndexPatterns },
@ -129,16 +131,26 @@ export const AddExceptionModal = memo(function AddExceptionModal({
);
const onError = useCallback(
(error: Error) => {
(error: Error): void => {
addError(error, { title: i18n.ADD_EXCEPTION_ERROR });
onCancel();
},
[addError, onCancel]
);
const onSuccess = useCallback(() => {
addSuccess(i18n.ADD_EXCEPTION_SUCCESS);
onConfirm(shouldCloseAlert, shouldBulkCloseAlert);
}, [addSuccess, onConfirm, shouldBulkCloseAlert, shouldCloseAlert]);
const onSuccess = useCallback(
(updated: number, conflicts: number): void => {
addSuccess(i18n.ADD_EXCEPTION_SUCCESS);
onConfirm(shouldCloseAlert, shouldBulkCloseAlert);
if (conflicts > 0) {
addWarning({
title: i18nCommon.UPDATE_ALERT_STATUS_FAILED(conflicts),
text: i18nCommon.UPDATE_ALERT_STATUS_FAILED_DETAILED(updated, conflicts),
});
}
},
[addSuccess, addWarning, onConfirm, shouldBulkCloseAlert, shouldCloseAlert]
);
const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException(
{
@ -153,7 +165,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
exceptionItems,
}: {
exceptionItems: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>;
}) => {
}): void => {
setExceptionItemsToAdd(exceptionItems);
},
[setExceptionItemsToAdd]
@ -186,7 +198,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
);
const handleFetchOrCreateExceptionListError = useCallback(
(error: Error, statusCode: number | null, message: string | null) => {
(error: Error, statusCode: number | null, message: string | null): void => {
setFetchOrCreateListError({
reason: error.message,
code: statusCode,
@ -205,7 +217,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
onSuccess: handleRuleChange,
});
const initialExceptionItems = useMemo(() => {
const initialExceptionItems = useMemo((): ExceptionsBuilderExceptionItem[] => {
if (exceptionListType === 'endpoint' && alertData !== undefined && ruleExceptionList) {
return defaultEndpointExceptionItems(
exceptionListType,
@ -218,7 +230,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
}
}, [alertData, exceptionListType, ruleExceptionList, ruleName]);
useEffect(() => {
useEffect((): void => {
if (isSignalIndexPatternLoading === false && isSignalIndexLoading === false) {
setShouldDisableBulkClose(
entryHasListType(exceptionItemsToAdd) ||
@ -234,34 +246,34 @@ export const AddExceptionModal = memo(function AddExceptionModal({
signalIndexPatterns,
]);
useEffect(() => {
useEffect((): void => {
if (shouldDisableBulkClose === true) {
setShouldBulkCloseAlert(false);
}
}, [shouldDisableBulkClose]);
const onCommentChange = useCallback(
(value: string) => {
(value: string): void => {
setComment(value);
},
[setComment]
);
const onCloseAlertCheckboxChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
(event: React.ChangeEvent<HTMLInputElement>): void => {
setShouldCloseAlert(event.currentTarget.checked);
},
[setShouldCloseAlert]
);
const onBulkCloseAlertCheckboxChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
(event: React.ChangeEvent<HTMLInputElement>): void => {
setShouldBulkCloseAlert(event.currentTarget.checked);
},
[setShouldBulkCloseAlert]
);
const retrieveAlertOsTypes = useCallback(() => {
const retrieveAlertOsTypes = useCallback((): string[] => {
const osDefaults = ['windows', 'macos'];
if (alertData) {
const osTypes = getMappedNonEcsValue({
@ -276,7 +288,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({
return osDefaults;
}, [alertData]);
const enrichExceptionItems = useCallback(() => {
const enrichExceptionItems = useCallback((): Array<
ExceptionListItemSchema | CreateExceptionListItemSchema
> => {
let enriched: Array<ExceptionListItemSchema | CreateExceptionListItemSchema> = [];
enriched =
comment !== ''
@ -289,7 +303,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
return enriched;
}, [comment, exceptionItemsToAdd, exceptionListType, retrieveAlertOsTypes]);
const onAddExceptionConfirm = useCallback(() => {
const onAddExceptionConfirm = useCallback((): void => {
if (addOrUpdateExceptionItems !== null) {
const alertIdToClose = shouldCloseAlert && alertData ? alertData.ecsData._id : undefined;
const bulkCloseIndex =
@ -306,7 +320,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
]);
const isSubmitButtonDisabled = useMemo(
() =>
(): boolean =>
fetchOrCreateListError != null ||
exceptionItemsToAdd.every((item) => item.entries.length === 0),
[fetchOrCreateListError, exceptionItemsToAdd]

2
x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.test.tsx
View file

@ -49,7 +49,7 @@ describe('useAddOrUpdateException', () => {
const onError = jest.fn();
const onSuccess = jest.fn();
const alertIdToClose = 'idToClose';
const bulkCloseIndex = ['.signals'];
const bulkCloseIndex = ['.custom'];
const itemsToAdd: CreateExceptionListItemSchema[] = [
{
...getCreateExceptionListItemSchemaMock(),

23
x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx
View file

@ -5,6 +5,7 @@
*/
import { useEffect, useRef, useState, useCallback } from 'react';
import { UpdateDocumentByQueryResponse } from 'elasticsearch';
import { HttpStart } from '../../../../../../../src/core/public';
import {
@ -43,7 +44,7 @@ export type ReturnUseAddOrUpdateException = [
export interface UseAddOrUpdateExceptionProps {
http: HttpStart;
onError: (arg: Error, code: number | null, message: string | null) => void;
onSuccess: () => void;
onSuccess: (updated: number, conficts: number) => void;
}
/**
@ -122,8 +123,10 @@ export const useAddOrUpdateException = ({
) => {
try {
setIsLoading(true);
if (alertIdToClose !== null && alertIdToClose !== undefined) {
await updateAlertStatus({
let alertIdResponse: UpdateDocumentByQueryResponse | undefined;
let bulkResponse: UpdateDocumentByQueryResponse | undefined;
if (alertIdToClose != null) {
alertIdResponse = await updateAlertStatus({
query: getUpdateAlertsQuery([alertIdToClose]),
status: 'closed',
signal: abortCtrl.signal,
@ -139,7 +142,8 @@ export const useAddOrUpdateException = ({
prepareExceptionItemsForBulkClose(exceptionItemsToAddOrUpdate),
false
);
await updateAlertStatus({
bulkResponse = await updateAlertStatus({
query: {
query: filter,
},
@ -150,9 +154,18 @@ export const useAddOrUpdateException = ({
await addOrUpdateItems(exceptionItemsToAddOrUpdate);
// NOTE: there could be some overlap here... it's possible that the first response had conflicts
// but that the alert was closed in the second call. In this case, a conflict will be reported even
// though it was already resolved. I'm not sure that there's an easy way to solve this, but it should
// have minimal impact on the user... they'd see a warning that indicates a possible conflict, but the
// state of the alerts and their representation in the UI would be consistent.
const updated = (alertIdResponse?.updated ?? 0) + (bulkResponse?.updated ?? 0);
const conflicts =
alertIdResponse?.version_conflicts ?? 0 + (bulkResponse?.version_conflicts ?? 0);
if (isSubscribed) {
setIsLoading(false);
onSuccess();
onSuccess(updated, conflicts);
}
} catch (error) {
if (isSubscribed) {

3
x-pack/plugins/security_solution/public/common/hooks/use_app_toasts.test.ts
View file

@ -14,13 +14,16 @@ jest.mock('../lib/kibana');
describe('useDeleteList', () => {
let addErrorMock: jest.Mock;
let addSuccessMock: jest.Mock;
let addWarningMock: jest.Mock;
beforeEach(() => {
addErrorMock = jest.fn();
addSuccessMock = jest.fn();
addWarningMock = jest.fn();
(useToasts as jest.Mock).mockImplementation(() => ({
addError: addErrorMock,
addSuccess: addSuccessMock,
addWarning: addWarningMock,
}));
});

5
x-pack/plugins/security_solution/public/common/hooks/use_app_toasts.ts
View file

@ -10,7 +10,7 @@ import { ErrorToastOptions, ToastsStart, Toast } from '../../../../../../src/cor
import { useToasts } from '../lib/kibana';
import { isAppError, AppError } from '../utils/api';
export type UseAppToasts = Pick<ToastsStart, 'addSuccess'> & {
export type UseAppToasts = Pick<ToastsStart, 'addSuccess' | 'addWarning'> & {
api: ToastsStart;
addError: (error: unknown, options: ErrorToastOptions) => Toast;
};
@ -19,6 +19,7 @@ export const useAppToasts = (): UseAppToasts => {
const toasts = useToasts();
const addError = useRef(toasts.addError.bind(toasts)).current;
const addSuccess = useRef(toasts.addSuccess.bind(toasts)).current;
const addWarning = useRef(toasts.addWarning.bind(toasts)).current;
const addAppError = useCallback(
(error: AppError, options: ErrorToastOptions) =>
@ -44,5 +45,5 @@ export const useAppToasts = (): UseAppToasts => {
[addAppError, addError]
);
return { api: toasts, addError: _addError, addSuccess };
return { api: toasts, addError: _addError, addSuccess, addWarning };
};

14
x-pack/plugins/security_solution/public/common/translations.ts
View file

@ -61,3 +61,17 @@ export const EMPTY_ACTION_ENDPOINT_DESCRIPTION = i18n.translate(
'Protect your hosts with threat prevention, detection, and deep security data visibility.',
}
);
export const UPDATE_ALERT_STATUS_FAILED = (conflicts: number) =>
i18n.translate('xpack.securitySolution.pages.common.updateAlertStatusFailed', {
values: { conflicts },
defaultMessage:
'Failed to update { conflicts } {conflicts, plural, =1 {alert} other {alerts}}.',
});
export const UPDATE_ALERT_STATUS_FAILED_DETAILED = (updated: number, conflicts: number) =>
i18n.translate('xpack.securitySolution.pages.common.updateAlertStatusFailedDetailed', {
values: { updated, conflicts },
defaultMessage: `{ updated } {updated, plural, =1 {alert was} other {alerts were}} updated successfully, but { conflicts } failed to update
because { conflicts, plural, =1 {it was} other {they were}} already being modified.`,
});

14
x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx
View file

@ -9,6 +9,7 @@
import dateMath from '@elastic/datemath';
import { get, getOr, isEmpty, find } from 'lodash/fp';
import moment from 'moment';
import { i18n } from '@kbn/i18n';
import { TimelineId } from '../../../../common/types/timeline';
import { updateAlertStatus } from '../../containers/detection_engine/alerts/api';
@ -83,7 +84,18 @@ export const updateAlertStatusAction = async ({
// TODO: Only delete those that were successfully updated from updatedRules
setEventsDeleted({ eventIds: alertIds, isDeleted: true });
onAlertStatusUpdateSuccess(response.updated, selectedStatus);
if (response.version_conflicts > 0 && alertIds.length === 1) {
throw new Error(
i18n.translate(
'xpack.securitySolution.detectionEngine.alerts.updateAlertStatusFailedSingleAlert',
{
defaultMessage: 'Failed to update alert because it was already being modified.',
}
)
);
}
onAlertStatusUpdateSuccess(response.updated, response.version_conflicts, selectedStatus);
} catch (error) {
onAlertStatusUpdateFailure(selectedStatus, error);
} finally {

38
x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx
View file

@ -9,10 +9,10 @@ import { isEmpty } from 'lodash/fp';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { connect, ConnectedProps } from 'react-redux';
import { Dispatch } from 'redux';
import { Status } from '../../../../common/detection_engine/schemas/common/schemas';
import { Filter, esQuery } from '../../../../../../../src/plugins/data/public';
import { TimelineIdLiteral } from '../../../../common/types/timeline';
import { useAppToasts } from '../../../common/hooks/use_app_toasts';
import { useFetchIndexPatterns } from '../../containers/detection_engine/rules/fetch_index_patterns';
import { StatefulEventsViewer } from '../../../common/components/events_viewer';
import { HeaderSection } from '../../../common/components/header_section';
@ -32,6 +32,7 @@ import {
} from './default_config';
import { FILTER_OPEN, AlertsTableFilterGroup } from './alerts_filter_group';
import { AlertsUtilityBar } from './alerts_utility_bar';
import * as i18nCommon from '../../../common/translations';
import * as i18n from './translations';
import {
SetEventsDeletedProps,
@ -90,6 +91,7 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
);
const kibana = useKibana();
const [, dispatchToaster] = useStateToaster();
const { addWarning } = useAppToasts();
const { initializeTimeline, setSelectAll, setIndexToAdd } = useManageTimeline();
const getGlobalQuery = useCallback(
@ -130,21 +132,29 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
);
const onAlertStatusUpdateSuccess = useCallback(
(count: number, status: Status) => {
let title: string;
switch (status) {
case 'closed':
title = i18n.CLOSED_ALERT_SUCCESS_TOAST(count);
break;
case 'open':
title = i18n.OPENED_ALERT_SUCCESS_TOAST(count);
break;
case 'in-progress':
title = i18n.IN_PROGRESS_ALERT_SUCCESS_TOAST(count);
(updated: number, conflicts: number, status: Status) => {
if (conflicts > 0) {
// Partial failure
addWarning({
title: i18nCommon.UPDATE_ALERT_STATUS_FAILED(conflicts),
text: i18nCommon.UPDATE_ALERT_STATUS_FAILED_DETAILED(updated, conflicts),
});
} else {
let title: string;
switch (status) {
case 'closed':
title = i18n.CLOSED_ALERT_SUCCESS_TOAST(updated);
break;
case 'open':
title = i18n.OPENED_ALERT_SUCCESS_TOAST(updated);
break;
case 'in-progress':
title = i18n.IN_PROGRESS_ALERT_SUCCESS_TOAST(updated);
}
displaySuccessToast(title, dispatchToaster);
}
displaySuccessToast(title, dispatchToaster);
},
[dispatchToaster]
[addWarning, dispatchToaster]
);
const onAlertStatusUpdateFailure = useCallback(

38
x-pack/plugins/security_solution/public/detections/components/alerts_table/timeline_actions/alert_context_menu.tsx
View file

@ -15,6 +15,7 @@ import {
} from '@elastic/eui';
import styled from 'styled-components';
import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
import { TimelineId } from '../../../../../common/types/timeline';
import { DEFAULT_INDEX_PATTERN } from '../../../../../common/constants';
import { Status, Type } from '../../../../../common/detection_engine/schemas/common/schemas';
@ -32,6 +33,7 @@ import {
AddExceptionModalBaseProps,
} from '../../../../common/components/exceptions/add_exception_modal';
import { getMappedNonEcsValue } from '../../../../common/components/exceptions/helpers';
import * as i18nCommon from '../../../../common/translations';
import * as i18n from '../translations';
import {
useStateToaster,
@ -72,6 +74,8 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
);
const eventId = ecsRowData._id;
const { addWarning } = useAppToasts();
const onButtonClick = useCallback(() => {
setPopover(!isPopoverOpen);
}, [isPopoverOpen]);
@ -124,22 +128,30 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
);
const onAlertStatusUpdateSuccess = useCallback(
(count: number, newStatus: Status) => {
let title: string;
switch (newStatus) {
case 'closed':
title = i18n.CLOSED_ALERT_SUCCESS_TOAST(count);
break;
case 'open':
title = i18n.OPENED_ALERT_SUCCESS_TOAST(count);
break;
case 'in-progress':
title = i18n.IN_PROGRESS_ALERT_SUCCESS_TOAST(count);
(updated: number, conflicts: number, newStatus: Status) => {
if (conflicts > 0) {
// Partial failure
addWarning({
title: i18nCommon.UPDATE_ALERT_STATUS_FAILED(conflicts),
text: i18nCommon.UPDATE_ALERT_STATUS_FAILED_DETAILED(updated, conflicts),
});
} else {
let title: string;
switch (newStatus) {
case 'closed':
title = i18n.CLOSED_ALERT_SUCCESS_TOAST(updated);
break;
case 'open':
title = i18n.OPENED_ALERT_SUCCESS_TOAST(updated);
break;
case 'in-progress':
title = i18n.IN_PROGRESS_ALERT_SUCCESS_TOAST(updated);
}
displaySuccessToast(title, dispatchToaster);
}
displaySuccessToast(title, dispatchToaster);
setAlertStatus(newStatus);
},
[dispatchToaster]
[dispatchToaster, addWarning]
);
const onAlertStatusUpdateFailure = useCallback(

2
x-pack/plugins/security_solution/public/detections/components/alerts_table/types.ts
View file

@ -44,7 +44,7 @@ export interface UpdateAlertStatusActionProps {
selectedStatus: Status;
setEventsLoading: ({ eventIds, isLoading }: SetEventsLoadingProps) => void;
setEventsDeleted: ({ eventIds, isDeleted }: SetEventsDeletedProps) => void;
onAlertStatusUpdateSuccess: (count: number, status: Status) => void;
onAlertStatusUpdateSuccess: (updated: number, conflicts: number, status: Status) => void;
onAlertStatusUpdateFailure: (status: Status, error: Error) => void;
}

4
x-pack/plugins/security_solution/public/detections/containers/detection_engine/alerts/api.test.ts
View file

@ -67,7 +67,7 @@ describe('Detections Alerts API', () => {
});
expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/signals/status', {
body:
'{"status":"closed","bool":{"filter":{"terms":{"_id":["b4ee5c32e3a321057edcc953ca17228c6fdfe5ba43fdbbdaffa8cefa11605cc5"]}}}}',
'{"conflicts":"proceed","status":"closed","bool":{"filter":{"terms":{"_id":["b4ee5c32e3a321057edcc953ca17228c6fdfe5ba43fdbbdaffa8cefa11605cc5"]}}}}',
method: 'POST',
signal: abortCtrl.signal,
});
@ -81,7 +81,7 @@ describe('Detections Alerts API', () => {
});
expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/signals/status', {
body:
'{"status":"open","bool":{"filter":{"terms":{"_id":["b4ee5c32e3a321057edcc953ca17228c6fdfe5ba43fdbbdaffa8cefa11605cc5"]}}}}',
'{"conflicts":"proceed","status":"open","bool":{"filter":{"terms":{"_id":["b4ee5c32e3a321057edcc953ca17228c6fdfe5ba43fdbbdaffa8cefa11605cc5"]}}}}',
method: 'POST',
signal: abortCtrl.signal,
});

2
x-pack/plugins/security_solution/public/detections/containers/detection_engine/alerts/api.ts
View file

@ -58,7 +58,7 @@ export const updateAlertStatus = async ({
}: UpdateAlertStatusProps): Promise<UpdateDocumentByQueryResponse> =>
KibanaServices.get().http.fetch(DETECTION_ENGINE_SIGNALS_STATUS_URL, {
method: 'POST',
body: JSON.stringify({ status, ...query }),
body: JSON.stringify({ conflicts: 'proceed', status, ...query }),
signal,
});

4
x-pack/plugins/security_solution/server/lib/detection_engine/routes/signals/open_close_signals_route.ts
View file

@ -28,11 +28,12 @@ export const setSignalsStatusRoute = (router: IRouter) => {
},
},
async (context, request, response) => {
const { signal_ids: signalIds, query, status } = request.body;
const { conflicts, signal_ids: signalIds, query, status } = request.body;
const clusterClient = context.core.elasticsearch.legacy.client;
const siemClient = context.securitySolution?.getAppClient();
const siemResponse = buildSiemResponse(response);
const validationErrors = setSignalStatusValidateTypeDependents(request.body);
if (validationErrors.length) {
return siemResponse.error({ statusCode: 400, body: validationErrors });
}
@ -55,6 +56,7 @@ export const setSignalsStatusRoute = (router: IRouter) => {
try {
const result = await clusterClient.callAsCurrentUser('updateByQuery', {
index: siemClient.getSignalsIndex(),
conflicts: conflicts ?? 'abort',
refresh: 'wait_for',
body: {
script: {