[Security Solution] [Timeline] Delete saved searches on timeline delete, prevent creating double saved searches (#174562)

## Summary

This pr fixes 2 issues currently present in the timeline es|ql via
discover integration: first, when timelines are deleted, the saved
object ids associated with those timelines that point to saved searches
are currently not deleted. If a user deletes a timeline and then tries
to create another with the same name, the esql is not able to be
persisted because no new saved search saved object can be created, as
the name of the old one collides with the new one. This pr fixes that by
deleting the saved search saved object on delete, although the issue
with timeline allowing multiple timelines with the same name while saved
search does not will remain an issue, need to use the onDuplicateTitle
callback that is currently a no-op to handle this here
https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/common/components/discover_in_timeline/use_discover_in_timeline_actions.tsx#L213
. Second, the use of react-query useMutation that creates the saved
search saved object will no longer create two, as it currently does, as
we check the status of the request in the useEffect that fires the
request. This might be a race condition bug with the saved search saved
objects client too, as normally objects are not supposed to be created
with the same name,
https://github.com/elastic/kibana/blob/main/src/plugins/saved_search/public/services/saved_searches/save_saved_searches.ts#L68,
but I'm not sure.

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
This commit is contained in:
Kevin Qualters 2024-01-12 12:10:20 -05:00 committed by GitHub
parent ad5c8cefee
commit 7b69611234
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 149 additions and 45 deletions

View file

@ -7,6 +7,10 @@
import * as rt from 'io-ts';
export const deleteTimelinesSchema = rt.type({
const searchId = rt.partial({ searchIds: rt.array(rt.string) });
const baseDeleteTimelinesSchema = rt.type({
savedObjectIds: rt.array(rt.string),
});
export const deleteTimelinesSchema = rt.intersection([baseDeleteTimelinesSchema, searchId]);

View file

@ -33,6 +33,10 @@ paths:
type: array
items:
type: string
searchId:
type: array
items:
type: string
responses:
200:
description: Indicates the timeline was successfully deleted.

View file

@ -48,7 +48,7 @@ export const useDiscoverInTimelineActions = (
const timeline = useShallowEqualSelector(
(state) => getTimeline(state, TimelineId.active) ?? timelineDefaults
);
const { savedSearchId } = timeline;
const { savedSearchId, version } = timeline;
// We're using a ref here to prevent a cyclic hook-dependency chain of updateSavedSearch
const timelineRef = useRef(timeline);
@ -56,7 +56,7 @@ export const useDiscoverInTimelineActions = (
const queryClient = useQueryClient();
const { mutateAsync: saveSavedSearch } = useMutation({
const { mutateAsync: saveSavedSearch, status } = useMutation({
mutationFn: ({
savedSearch,
savedSearchOptions,
@ -75,6 +75,7 @@ export const useDiscoverInTimelineActions = (
}
queryClient.invalidateQueries({ queryKey: ['savedSearchById', savedSearchId] });
},
mutationKey: [version],
});
const getDefaultDiscoverAppState: () => Promise<DiscoverAppState> = useCallback(async () => {
@ -217,7 +218,7 @@ export const useDiscoverInTimelineActions = (
const responseIsEmpty = !response || !response?.id;
if (responseIsEmpty) {
throw new Error('Response is empty');
} else if (!savedSearchId && !responseIsEmpty) {
} else if (!savedSearchId && !responseIsEmpty && status !== 'loading') {
dispatch(
timelineActions.updateSavedSearchId({
id: TimelineId.active,
@ -236,7 +237,7 @@ export const useDiscoverInTimelineActions = (
}
}
},
[persistSavedSearch, savedSearchId, dispatch, discoverDataService]
[persistSavedSearch, savedSearchId, dispatch, discoverDataService, status]
);
const initializeLocalSavedSearch = useCallback(

View file

@ -27,13 +27,14 @@ interface Props {
onComplete?: () => void;
isModalOpen: boolean;
savedObjectIds: string[];
savedSearchIds?: string[];
title: string | null;
}
/**
* Renders a button that when clicked, displays the `Delete Timeline` modal
*/
export const DeleteTimelineModalOverlay = React.memo<Props>(
({ deleteTimelines, isModalOpen, savedObjectIds, title, onComplete }) => {
({ deleteTimelines, isModalOpen, savedObjectIds, title, onComplete, savedSearchIds }) => {
const { addSuccess } = useAppToasts();
const { tabName: timelineType } = useParams<{ tabName: TimelineType }>();
@ -43,9 +44,16 @@ export const DeleteTimelineModalOverlay = React.memo<Props>(
}
}, [onComplete]);
const onDelete = useCallback(() => {
if (savedObjectIds.length > 0) {
if (savedObjectIds.length > 0 && savedSearchIds != null && savedSearchIds.length > 0) {
deleteTimelines(savedObjectIds, savedSearchIds);
addSuccess({
title:
timelineType === TimelineType.template
? i18n.SUCCESSFULLY_DELETED_TIMELINE_TEMPLATES(savedObjectIds.length)
: i18n.SUCCESSFULLY_DELETED_TIMELINES(savedObjectIds.length),
});
} else if (savedObjectIds.length > 0) {
deleteTimelines(savedObjectIds);
addSuccess({
title:
timelineType === TimelineType.template
@ -53,10 +61,11 @@ export const DeleteTimelineModalOverlay = React.memo<Props>(
: i18n.SUCCESSFULLY_DELETED_TIMELINES(savedObjectIds.length),
});
}
if (onComplete != null) {
onComplete();
}
}, [deleteTimelines, savedObjectIds, onComplete, addSuccess, timelineType]);
}, [deleteTimelines, savedObjectIds, onComplete, addSuccess, timelineType, savedSearchIds]);
return (
<>
{isModalOpen && <RemovePopover data-test-subj="remove-popover" />}

View file

@ -15,14 +15,7 @@ import * as i18n from './translations';
import type { DeleteTimelines, OpenTimelineResult } from './types';
import { EditTimelineActions } from './export_timeline';
import { useEditTimelineActions } from './edit_timeline_actions';
const getExportedIds = (selectedTimelines: OpenTimelineResult[]) => {
const array = Array.isArray(selectedTimelines) ? selectedTimelines : [selectedTimelines];
return array.reduce(
(acc, item) => (item.savedObjectId != null ? [...acc, item.savedObjectId] : [...acc]),
[] as string[]
);
};
import { getSelectedTimelineIdsAndSearchIds, getRequestIds } from '.';
export const useEditTimelineBatchActions = ({
deleteTimelines,
@ -56,7 +49,13 @@ export const useEditTimelineBatchActions = ({
[disableExportTimelineDownloader, onCloseDeleteTimelineModal, tableRef]
);
const selectedIds = useMemo(() => getExportedIds(selectedItems ?? []), [selectedItems]);
const { timelineIds, searchIds } = useMemo(() => {
if (selectedItems != null) {
return getRequestIds(getSelectedTimelineIdsAndSearchIds(selectedItems));
} else {
return { timelineIds: [], searchIds: undefined };
}
}, [selectedItems]);
const handleEnableExportTimelineDownloader = useCallback(
() => enableExportTimelineDownloader(),
@ -102,7 +101,8 @@ export const useEditTimelineBatchActions = ({
<>
<EditTimelineActions
deleteTimelines={deleteTimelines}
ids={selectedIds}
ids={timelineIds}
savedSearchIds={searchIds}
isEnableDownloader={isEnableDownloader}
isDeleteTimelineModalOpen={isDeleteTimelineModalOpen}
onComplete={onCompleteBatchActions.bind(null, closePopover)}
@ -121,7 +121,8 @@ export const useEditTimelineBatchActions = ({
[
selectedItems,
deleteTimelines,
selectedIds,
timelineIds,
searchIds,
isEnableDownloader,
isDeleteTimelineModalOpen,
onCompleteBatchActions,

View file

@ -20,6 +20,7 @@ export interface ExportTimeline {
export const EditTimelineActionsComponent: React.FC<{
deleteTimelines: DeleteTimelines | undefined;
ids: string[];
savedSearchIds?: string[];
isEnableDownloader: boolean;
isDeleteTimelineModalOpen: boolean;
onComplete: () => void;
@ -27,6 +28,7 @@ export const EditTimelineActionsComponent: React.FC<{
}> = ({
deleteTimelines,
ids,
savedSearchIds,
isEnableDownloader,
isDeleteTimelineModalOpen,
onComplete,
@ -46,6 +48,7 @@ export const EditTimelineActionsComponent: React.FC<{
isModalOpen={isDeleteTimelineModalOpen}
onComplete={onComplete}
savedObjectIds={ids}
savedSearchIds={savedSearchIds}
title={title}
/>
)}

View file

@ -74,14 +74,51 @@ export type OpenTimelineOwnProps = OwnProps &
>;
/** Returns a collection of selected timeline ids */
export const getSelectedTimelineIds = (selectedItems: OpenTimelineResult[]): string[] =>
selectedItems.reduce<string[]>(
(validSelections, timelineResult) =>
timelineResult.savedObjectId != null
? [...validSelections, timelineResult.savedObjectId]
: validSelections,
[]
export const getSelectedTimelineIdsAndSearchIds = (
selectedItems: OpenTimelineResult[]
): Array<{ timelineId: string; searchId?: string | null }> => {
return selectedItems.reduce<Array<{ timelineId: string; searchId?: string | null }>>(
(validSelections, timelineResult) => {
if (timelineResult.savedObjectId != null && timelineResult.savedSearchId != null) {
return [
...validSelections,
{ timelineId: timelineResult.savedObjectId, searchId: timelineResult.savedSearchId },
];
} else if (timelineResult.savedObjectId != null) {
return [...validSelections, { timelineId: timelineResult.savedObjectId }];
} else {
return validSelections;
}
},
[] as Array<{ timelineId: string; searchId?: string | null }>
);
};
interface DeleteTimelinesValues {
timelineIds: string[];
searchIds: string[];
}
export const getRequestIds = (
timelineIdsWithSearch: Array<{ timelineId: string; searchId?: string | null }>
) => {
return timelineIdsWithSearch.reduce<DeleteTimelinesValues>(
(acc, { timelineId, searchId }) => {
let requestValues = acc;
if (searchId != null) {
requestValues = { ...requestValues, searchIds: [...requestValues.searchIds, searchId] };
}
if (timelineId != null) {
requestValues = {
...requestValues,
timelineIds: [...requestValues.timelineIds, timelineId],
};
}
return requestValues;
},
{ timelineIds: [], searchIds: [] }
);
};
/** Manages the state (e.g table selection) of the (pure) `OpenTimeline` component */
// eslint-disable-next-line react/display-name
@ -208,7 +245,7 @@ export const StatefulOpenTimelineComponent = React.memo<OpenTimelineOwnProps>(
// };
const deleteTimelines: DeleteTimelines = useCallback(
async (timelineIds: string[]) => {
async (timelineIds: string[], searchIds?: string[]) => {
startTransaction({
name: timelineIds.length > 1 ? TIMELINE_ACTIONS.BULK_DELETE : TIMELINE_ACTIONS.DELETE,
});
@ -225,16 +262,16 @@ export const StatefulOpenTimelineComponent = React.memo<OpenTimelineOwnProps>(
);
}
await deleteTimelinesByIds(timelineIds);
await deleteTimelinesByIds(timelineIds, searchIds);
refetch();
},
[startTransaction, timelineSavedObjectId, refetch, dispatch, dataViewId, selectedPatterns]
);
const onDeleteOneTimeline: OnDeleteOneTimeline = useCallback(
async (timelineIds: string[]) => {
async (timelineIds: string[], searchIds?: string[]) => {
// The type for `deleteTimelines` is incorrect, it returns a Promise
await deleteTimelines(timelineIds);
await deleteTimelines(timelineIds, searchIds);
},
[deleteTimelines]
);
@ -242,7 +279,9 @@ export const StatefulOpenTimelineComponent = React.memo<OpenTimelineOwnProps>(
/** Invoked when the user clicks the action to delete the selected timelines */
const onDeleteSelected: OnDeleteSelected = useCallback(async () => {
// The type for `deleteTimelines` is incorrect, it returns a Promise
await deleteTimelines(getSelectedTimelineIds(selectedItems));
const timelineIdsWithSearch = getSelectedTimelineIdsAndSearchIds(selectedItems);
const { timelineIds, searchIds } = getRequestIds(timelineIdsWithSearch);
await deleteTimelines(timelineIds, searchIds);
// NOTE: we clear the selection state below, but if the server fails to
// delete a timeline, it will remain selected in the table:

View file

@ -129,6 +129,12 @@ export const OpenTimeline = React.memo<OpenTimelineProps>(
[actionItem]
);
const actionItemSavedSearchId = useMemo(() => {
return actionItem != null && actionItem.savedSearchId != null
? [actionItem.savedSearchId]
: undefined;
}, [actionItem]);
const onRefreshBtnClick = useCallback(() => {
if (refetch != null) {
refetch();
@ -197,6 +203,7 @@ export const OpenTimeline = React.memo<OpenTimelineProps>(
<EditTimelineActions
deleteTimelines={deleteTimelines}
ids={actionItemId}
savedSearchIds={actionItemSavedSearchId}
isDeleteTimelineModalOpen={isDeleteTimelineModalOpen}
isEnableDownloader={isEnableDownloader}
onComplete={onCompleteEditTimelineAction}

View file

@ -6,7 +6,6 @@
*/
import type React from 'react';
import type { AllTimelinesVariables } from '../../containers/all';
import type { TimelineModel } from '../../store/model';
import type {
RowRendererId,
@ -59,6 +58,7 @@ export interface OpenTimelineResult {
pinnedEventIds?: Readonly<Record<string, boolean>> | null;
queryType?: { hasEql: boolean; hasQuery: boolean };
savedObjectId?: string | null;
savedSearchId?: string | null;
status?: TimelineStatus | null;
title?: string | null;
templateTimelineId?: string | null;
@ -77,7 +77,7 @@ export interface EuiSearchBarQuery {
}
/** Performs IO to delete the specified timelines */
export type DeleteTimelines = (timelineIds: string[], variables?: AllTimelinesVariables) => void;
export type DeleteTimelines = (timelineIds: string[], searchIds?: string[]) => void;
/** Invoked when the user clicks the action create rule from timeline */
export type OnCreateRuleFromTimeline = (savedObjectId: string) => void;

View file

@ -88,6 +88,7 @@ export const getAllTimeline = memoizeOne(
)
: null,
savedObjectId: timeline.savedObjectId,
savedSearchId: timeline.savedSearchId,
status: timeline.status,
title: timeline.title,
updated: timeline.updated,

View file

@ -480,13 +480,20 @@ export const persistFavorite = async ({
return decodeResponseFavoriteTimeline(response);
};
export const deleteTimelinesByIds = async (savedObjectIds: string[]) => {
export const deleteTimelinesByIds = async (savedObjectIds: string[], searchIds?: string[]) => {
let requestBody;
try {
requestBody = JSON.stringify({
savedObjectIds,
});
if (searchIds) {
requestBody = JSON.stringify({
savedObjectIds,
searchIds,
});
} else {
requestBody = JSON.stringify({
savedObjectIds,
});
}
} catch (err) {
return Promise.reject(new Error(`Failed to stringify query: ${JSON.stringify(err)}`));
}

View file

@ -42,9 +42,9 @@ export const deleteTimelinesRoute = (
try {
const frameworkRequest = await buildFrameworkRequest(context, security, request);
const { savedObjectIds } = request.body;
const { savedObjectIds, searchIds } = request.body;
await deleteTimeline(frameworkRequest, savedObjectIds);
await deleteTimeline(frameworkRequest, savedObjectIds, searchIds);
return response.ok({ body: { data: { deleteTimeline: true } } });
} catch (err) {
const error = transformError(err);

View file

@ -0,0 +1,22 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import type { FrameworkRequest } from '../../../framework';
export const deleteSearchByTimelineId = async (
request: FrameworkRequest,
savedSearchIds?: string[]
) => {
if (savedSearchIds !== undefined) {
const savedObjectsClient = (await request.context.core).savedObjects.client;
const objects = savedSearchIds.map((id) => ({ id, type: 'search' }));
await savedObjectsClient.bulkDelete(objects);
} else {
return Promise.resolve();
}
};

View file

@ -38,6 +38,7 @@ import type { SavedObjectTimelineWithoutExternalRefs } from '../../../../../comm
import type { FrameworkRequest } from '../../../framework';
import * as note from '../notes/saved_object';
import * as pinnedEvent from '../pinned_events';
import { deleteSearchByTimelineId } from '../saved_search';
import { convertSavedObjectToSavedTimeline } from './convert_saved_object_to_savedtimeline';
import { pickSavedTimeline } from './pick_saved_timeline';
import { timelineSavedObjectType } from '../../saved_object_mappings';
@ -572,18 +573,23 @@ export const resetTimeline = async (
return response;
};
export const deleteTimeline = async (request: FrameworkRequest, timelineIds: string[]) => {
export const deleteTimeline = async (
request: FrameworkRequest,
timelineIds: string[],
searchIds?: string[]
) => {
const savedObjectsClient = (await request.context.core).savedObjects.client;
await Promise.all(
timelineIds.map((timelineId) =>
await Promise.all([
...timelineIds.map((timelineId) =>
Promise.all([
savedObjectsClient.delete(timelineSavedObjectType, timelineId),
note.deleteNoteByTimelineId(request, timelineId),
pinnedEvent.deleteAllPinnedEventsOnTimeline(request, timelineId),
])
)
);
),
deleteSearchByTimelineId(request, searchIds),
]);
};
export const copyTimeline = async (