Skip to content

Commit

Permalink
Cases delete files case deletion (#153979)
Browse files Browse the repository at this point in the history
This PR extends the case deletion functionality to delete any files that
are attached to the case when a case is deleted.

To avoid attempting to delete too many files at once I chunk case ids
into 50 at a time such that we're at most only deleting 5000 (50 case *
100 files per case) at once. That way we don't exceed the 10k find api
limit.

## Testing
Run the python script from here:
https://github.com/elastic/cases-files-generator to generate some files
for a case

Deleting the case should remove all files
  • Loading branch information
jonathan-buttner authored Apr 10, 2023
1 parent c98fa82 commit b6a113c
Show file tree
Hide file tree
Showing 12 changed files with 568 additions and 46 deletions.
2 changes: 1 addition & 1 deletion src/plugins/files/server/file_service/file_action_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,5 @@ export interface FindFileArgs extends Pagination {
/**
* File metadata values. These values are governed by the consumer.
*/
meta?: Record<string, string>;
meta?: Record<string, string | string[]>;
}
5 changes: 5 additions & 0 deletions x-pack/plugins/cases/common/api/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
INTERNAL_CONNECTORS_URL,
INTERNAL_CASE_USERS_URL,
INTERNAL_DELETE_FILE_ATTACHMENTS_URL,
CASE_FIND_ATTACHMENTS_URL,
} from '../constants';

export const getCaseDetailsUrl = (id: string): string => {
Expand All @@ -39,6 +40,10 @@ export const getCaseCommentDetailsUrl = (caseId: string, commentId: string): str
return CASE_COMMENT_DETAILS_URL.replace('{case_id}', caseId).replace('{comment_id}', commentId);
};

export const getCaseFindAttachmentsUrl = (caseId: string): string => {
return CASE_FIND_ATTACHMENTS_URL.replace('{case_id}', caseId);
};

export const getCaseCommentDeleteUrl = (caseId: string, commentId: string): string => {
return CASE_COMMENT_DELETE_URL.replace('{case_id}', caseId).replace('{comment_id}', commentId);
};
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/common/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const CASE_CONFIGURE_DETAILS_URL = `${CASES_URL}/configure/{configuration
export const CASE_CONFIGURE_CONNECTORS_URL = `${CASE_CONFIGURE_URL}/connectors` as const;

export const CASE_COMMENTS_URL = `${CASE_DETAILS_URL}/comments` as const;
export const CASE_FIND_ATTACHMENTS_URL = `${CASE_COMMENTS_URL}/_find` as const;
export const CASE_COMMENT_DETAILS_URL = `${CASE_DETAILS_URL}/comments/{comment_id}` as const;
export const CASE_COMMENT_DELETE_URL = `${CASE_DETAILS_URL}/comments/{comment_id}` as const;
export const CASE_PUSH_URL = `${CASE_DETAILS_URL}/connector/{connector_id}/_push` as const;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/common/files/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const CaseFileMetadataForDeletionRt = rt.type({
caseIds: rt.array(rt.string),
});

export type CaseFileMetadata = rt.TypeOf<typeof CaseFileMetadataForDeletionRt>;
export type CaseFileMetadataForDeletion = rt.TypeOf<typeof CaseFileMetadataForDeletionRt>;

const FILE_KIND_DELIMITER = 'FilesCases';

Expand Down
34 changes: 6 additions & 28 deletions x-pack/plugins/cases/server/client/attachments/bulk_delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ import { identity } from 'fp-ts/lib/function';

import pMap from 'p-map';
import { partition } from 'lodash';
import type { File } from '@kbn/files-plugin/common';
import type { File, FileJSON } from '@kbn/files-plugin/common';
import type { FileServiceStart } from '@kbn/files-plugin/server';
import { FileNotFoundError } from '@kbn/files-plugin/server/file_service/errors';
import { BulkDeleteFileAttachmentsRequestRt, excess, throwErrors } from '../../../common/api';
import { MAX_CONCURRENT_SEARCHES } from '../../../common/constants';
import type { CasesClientArgs } from '../types';
import { createCaseError } from '../../common/error';
import type { OwnerEntity } from '../../authorization';
import { Operations } from '../../authorization';
import type { BulkDeleteFileArgs } from './types';
import { constructOwnerFromFileKind, CaseFileMetadataForDeletionRt } from '../../../common/files';
import { CaseFileMetadataForDeletionRt } from '../../../common/files';
import type { CasesClient } from '../client';
import { createFileEntities, deleteFiles } from '../files';

export const bulkDeleteFileAttachments = async (
{ caseId, fileIds }: BulkDeleteFileArgs,
Expand Down Expand Up @@ -67,9 +67,7 @@ export const bulkDeleteFileAttachments = async (
});

await Promise.all([
pMap(request.ids, async (fileId: string) => fileService.delete({ id: fileId }), {
concurrency: MAX_CONCURRENT_SEARCHES,
}),
deleteFiles(request.ids, fileService),
attachmentService.bulkDelete({
attachmentIds: fileAttachments.map((so) => so.id),
refresh: false,
Expand Down Expand Up @@ -117,7 +115,7 @@ const getFiles = async (
caseId: BulkDeleteFileArgs['caseId'],
fileIds: BulkDeleteFileArgs['fileIds'],
fileService: FileServiceStart
) => {
): Promise<FileJSON[]> => {
// it's possible that we're trying to delete a file when an attachment wasn't created (for example if the create
// attachment request failed)
const files = await pMap(fileIds, async (fileId: string) => fileService.getById({ id: fileId }), {
Expand All @@ -143,25 +141,5 @@ const getFiles = async (
throw Boom.badRequest('Failed to find files to delete');
}

return validFiles;
};

const createFileEntities = (files: File[]) => {
const fileEntities: OwnerEntity[] = [];

// It's possible that the owner array could have invalid information in it so we'll use the file kind for determining if the user
// has the correct authorization for deleting these files
for (const fileInfo of files) {
const ownerFromFileKind = constructOwnerFromFileKind(fileInfo.data.fileKind);

if (ownerFromFileKind == null) {
throw Boom.badRequest(
`File id ${fileInfo.id} has invalid file kind ${fileInfo.data.fileKind}`
);
}

fileEntities.push({ id: fileInfo.id, owner: ownerFromFileKind });
}

return fileEntities;
return validFiles.map((fileInfo) => fileInfo.data);
};
79 changes: 79 additions & 0 deletions x-pack/plugins/cases/server/client/cases/delete.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* 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 { MAX_FILES_PER_CASE } from '../../../common/constants';
import type { FindFileArgs } from '@kbn/files-plugin/server';
import { createFileServiceMock } from '@kbn/files-plugin/server/mocks';
import type { FileJSON } from '@kbn/shared-ux-file-types';
import type { CaseFileMetadataForDeletion } from '../../../common/files';
import { constructFileKindIdByOwner } from '../../../common/files';
import { getFileEntities } from './delete';

const getCaseIds = (numIds: number) => {
return Array.from(Array(numIds).keys()).map((key) => key.toString());
};
describe('delete', () => {
describe('getFileEntities', () => {
const numCaseIds = 1000;
const caseIds = getCaseIds(numCaseIds);
const mockFileService = createFileServiceMock();
mockFileService.find.mockImplementation(async (args: FindFileArgs) => {
const caseMeta = args.meta as unknown as CaseFileMetadataForDeletion;
const numFilesToGen = caseMeta.caseIds.length * MAX_FILES_PER_CASE;
const files = Array.from(Array(numFilesToGen).keys()).map(() => createMockFileJSON());

return {
files,
total: files.length,
};
});

beforeEach(() => {
jest.clearAllMocks();
});

it('only provides 50 case ids in a single call to the find api', async () => {
await getFileEntities(caseIds, mockFileService);

for (const call of mockFileService.find.mock.calls) {
const callMeta = call[0].meta as unknown as CaseFileMetadataForDeletion;
expect(callMeta.caseIds.length).toEqual(50);
}
});

it('calls the find function the number of case ids divided by the chunk size', async () => {
await getFileEntities(caseIds, mockFileService);

const chunkSize = 50;

expect(mockFileService.find).toHaveBeenCalledTimes(numCaseIds / chunkSize);
});

it('returns the number of entities equal to the case ids times the max files per case limit', async () => {
const expectedEntities = Array.from(Array(numCaseIds * MAX_FILES_PER_CASE).keys()).map(
() => ({
id: '123',
owner: 'securitySolution',
})
);

const entities = await getFileEntities(caseIds, mockFileService);

expect(entities.length).toEqual(numCaseIds * MAX_FILES_PER_CASE);
expect(entities).toEqual(expectedEntities);
});
});
});

const createMockFileJSON = (): FileJSON => {
return {
id: '123',
fileKind: constructFileKindIdByOwner('securitySolution'),
meta: {
owner: ['securitySolution'],
},
} as unknown as FileJSON;
};
51 changes: 44 additions & 7 deletions x-pack/plugins/cases/server/client/cases/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,32 @@
*/

import { Boom } from '@hapi/boom';
import pMap from 'p-map';
import { chunk } from 'lodash';
import type { SavedObjectsBulkDeleteObject } from '@kbn/core/server';
import type { FileServiceStart } from '@kbn/files-plugin/server';
import {
CASE_COMMENT_SAVED_OBJECT,
CASE_SAVED_OBJECT,
CASE_USER_ACTION_SAVED_OBJECT,
MAX_FILES_PER_CASE,
MAX_DOCS_PER_PAGE,
} from '../../../common/constants';
import type { CasesClientArgs } from '..';
import { createCaseError } from '../../common/error';
import type { OwnerEntity } from '../../authorization';
import { Operations } from '../../authorization';
import { createFileEntities, deleteFiles } from '../files';

/**
* Deletes the specified cases and their attachments.
*
* @ignore
*/
export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): Promise<void> {
const {
services: { caseService, attachmentService, userActionService },
logger,
authorization,
fileService,
} = clientArgs;
try {
const cases = await caseService.getCases({ caseIds: ids });
Expand All @@ -44,9 +49,11 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P
entities.set(theCase.id, { id: theCase.id, owner: theCase.attributes.owner });
}

const fileEntities = await getFileEntities(ids, fileService);

await authorization.ensureAuthorized({
operation: Operations.deleteCase,
entities: Array.from(entities.values()),
entities: [...Array.from(entities.values()), ...fileEntities],
});

const attachmentIds = await attachmentService.getter.getAttachmentIdsForCases({
Expand All @@ -61,10 +68,14 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P
...userActionIds.map((id) => ({ id, type: CASE_USER_ACTION_SAVED_OBJECT })),
];

await caseService.bulkDeleteCaseEntities({
entities: bulkDeleteEntities,
options: { refresh: 'wait_for' },
});
const fileIds = fileEntities.map((entity) => entity.id);
await Promise.all([
deleteFiles(fileIds, fileService),
caseService.bulkDeleteCaseEntities({
entities: bulkDeleteEntities,
options: { refresh: 'wait_for' },
}),
]);

await userActionService.creator.bulkAuditLogCaseDeletion(
cases.saved_objects.map((caseInfo) => caseInfo.id)
Expand All @@ -77,3 +88,29 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P
});
}
}

export const getFileEntities = async (
caseIds: string[],
fileService: FileServiceStart
): Promise<OwnerEntity[]> => {
// using 50 just to be safe, each case can have 100 files = 50 * 100 = 5000 which is half the max number of docs that
// the client can request
const chunkSize = MAX_FILES_PER_CASE / 2;
const chunkedIds = chunk(caseIds, chunkSize);

const entityResults = await pMap(chunkedIds, async (ids: string[]) => {
const findRes = await fileService.find({
perPage: MAX_DOCS_PER_PAGE,
meta: {
caseIds: ids,
},
});

const fileEntities = createFileEntities(findRes.files);
return fileEntities;
});

const entities = entityResults.flatMap((res) => res);

return entities;
};
77 changes: 77 additions & 0 deletions x-pack/plugins/cases/server/client/files/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* 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 { createFileServiceMock } from '@kbn/files-plugin/server/mocks';
import { OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER } from '../../../common';
import { constructFileKindIdByOwner } from '../../../common/files';
import { createFileEntities, deleteFiles } from '.';

describe('server files', () => {
describe('createFileEntities', () => {
it('returns an empty array when passed no files', () => {
expect(createFileEntities([])).toEqual([]);
});

it('throws an error when the file kind is not valid', () => {
expect.assertions(1);

expect(() =>
createFileEntities([{ fileKind: 'abc', id: '1' }])
).toThrowErrorMatchingInlineSnapshot(`"File id 1 has invalid file kind abc"`);
});

it('throws an error when one of the file entities does not have a valid file kind', () => {
expect.assertions(1);

expect(() =>
createFileEntities([
{ fileKind: constructFileKindIdByOwner(SECURITY_SOLUTION_OWNER), id: '1' },
{ fileKind: 'abc', id: '2' },
])
).toThrowErrorMatchingInlineSnapshot(`"File id 2 has invalid file kind abc"`);
});

it('returns an array of entities when the file kind is valid', () => {
expect.assertions(1);

expect(
createFileEntities([
{ fileKind: constructFileKindIdByOwner(SECURITY_SOLUTION_OWNER), id: '1' },
{ fileKind: constructFileKindIdByOwner(OBSERVABILITY_OWNER), id: '2' },
])
).toEqual([
{ id: '1', owner: 'securitySolution' },
{ id: '2', owner: 'observability' },
]);
});
});

describe('deleteFiles', () => {
it('calls delete twice with the ids passed in', async () => {
const fileServiceMock = createFileServiceMock();

expect.assertions(2);
await deleteFiles(['1', '2'], fileServiceMock);

expect(fileServiceMock.delete).toBeCalledTimes(2);
expect(fileServiceMock.delete.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"id": "1",
},
],
Array [
Object {
"id": "2",
},
],
]
`);
});
});
});
Loading

0 comments on commit b6a113c

Please sign in to comment.