From b12238bac8d0614d675c3a90fa30a8dd733e5389 Mon Sep 17 00:00:00 2001 From: Antonio Date: Fri, 30 Jun 2023 14:40:12 +0200 Subject: [PATCH] [Cases] Delete Cases API Guardrails (#160846) Connected to #146945 ## Summary | Description | Limit | Done? | Documented? | ------------- | ---- | :---: | ---- | | Total number of cases to be deleted | 100 | :white_check_mark: | Yes | - Used schema validation. - Updated documentation. - Added jest and e2e tests. ### Checklist Delete any items that are not applicable to this PR. - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [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 ### Release notes The Delete cases API now limits the number of cases to be deleted to 100. --- x-pack/plugins/cases/common/api/cases/case.ts | 11 ++++++++++- x-pack/plugins/cases/common/constants/index.ts | 1 + x-pack/plugins/cases/docs/openapi/bundled.json | 7 ++++++- x-pack/plugins/cases/docs/openapi/bundled.yaml | 6 +++++- .../openapi/components/parameters/ids.yaml | 11 ++++++----- .../openapi/paths/s@{spaceid}@api@cases.yaml | 8 ++++---- .../cases/server/client/cases/delete.test.ts | 18 +++++++++++++++++- .../cases/server/client/cases/delete.ts | 10 ++++++++-- .../tests/common/cases/delete_cases.ts | 16 ++++++++++++++++ 9 files changed, 73 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index acfca22e70498..7fcbc86e3d340 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -13,12 +13,13 @@ import { CommentRt } from './comment'; import { CasesStatusResponseRt, CaseStatusRt } from './status'; import { CaseConnectorRt } from '../connectors/connector'; import { CaseAssigneesRt } from './assignee'; +import { limitedArraySchema, NonEmptyString } from '../../schema'; import { + MAX_DELETE_IDS_LENGTH, MAX_ASSIGNEES_FILTER_LENGTH, MAX_REPORTERS_FILTER_LENGTH, MAX_TAGS_FILTER_LENGTH, } from '../../constants'; -import { limitedArraySchema } from '../../schema'; export const AttachmentTotalsRt = rt.strict({ alerts: rt.number, @@ -295,6 +296,13 @@ export const CasesFindRequestRt = rt.exact( }) ); +export const CasesDeleteRequestRt = limitedArraySchema( + NonEmptyString, + 1, + MAX_DELETE_IDS_LENGTH, + 'ids' +); + export const CasesByAlertIDRequestRt = rt.exact( rt.partial({ /** @@ -432,6 +440,7 @@ export type CasePostRequest = rt.TypeOf; export type Case = rt.TypeOf; export type CaseResolveResponse = rt.TypeOf; export type Cases = rt.TypeOf; +export type CasesDeleteRequest = rt.TypeOf; export type CasesFindRequest = rt.TypeOf; export type CasesByAlertIDRequest = rt.TypeOf; export type CasesFindResponse = rt.TypeOf; diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts index 24365cdc625e8..d41efe9e8d85e 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -116,6 +116,7 @@ export const MAX_REPORTERS_FILTER_LENGTH = 100 as const; export const MAX_TITLE_LENGTH = 160 as const; export const MAX_CATEGORY_LENGTH = 50 as const; +export const MAX_DELETE_IDS_LENGTH = 100 as const; /** * Cases features diff --git a/x-pack/plugins/cases/docs/openapi/bundled.json b/x-pack/plugins/cases/docs/openapi/bundled.json index 2ce8031bc1cb9..641db779ff346 100644 --- a/x-pack/plugins/cases/docs/openapi/bundled.json +++ b/x-pack/plugins/cases/docs/openapi/bundled.json @@ -3809,7 +3809,12 @@ "in": "query", "required": true, "schema": { - "type": "string" + "type": "array", + "items": { + "type": "string", + "minItems": 1, + "maxItems": 100 + } }, "example": "d4e7abb0-b462-11ec-9a8d-698504725a43" }, diff --git a/x-pack/plugins/cases/docs/openapi/bundled.yaml b/x-pack/plugins/cases/docs/openapi/bundled.yaml index 50184a7b96742..bf961003f706f 100644 --- a/x-pack/plugins/cases/docs/openapi/bundled.yaml +++ b/x-pack/plugins/cases/docs/openapi/bundled.yaml @@ -2310,7 +2310,11 @@ components: in: query required: true schema: - type: string + type: array + items: + type: string + minItems: 1 + maxItems: 100 example: d4e7abb0-b462-11ec-9a8d-698504725a43 assignees: in: query diff --git a/x-pack/plugins/cases/docs/openapi/components/parameters/ids.yaml b/x-pack/plugins/cases/docs/openapi/components/parameters/ids.yaml index 1690b5e6038a0..c84ec64ab2a53 100644 --- a/x-pack/plugins/cases/docs/openapi/components/parameters/ids.yaml +++ b/x-pack/plugins/cases/docs/openapi/components/parameters/ids.yaml @@ -5,8 +5,9 @@ description: > in: query required: true schema: - type: string -example: d4e7abb0-b462-11ec-9a8d-698504725a43 - - - \ No newline at end of file + type: array + items: + type: string + minItems: 1 + maxItems: 100 +example: d4e7abb0-b462-11ec-9a8d-698504725a43 diff --git a/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases.yaml b/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases.yaml index ae228f7c1f770..58e73f6a68d17 100644 --- a/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases.yaml +++ b/x-pack/plugins/cases/docs/openapi/paths/s@{spaceid}@api@cases.yaml @@ -23,7 +23,7 @@ post: '200': description: Indicates a successful call. content: - application/json: + application/json: schema: $ref: '../components/schemas/case_response_properties.yaml' examples: @@ -36,7 +36,7 @@ post: schema: $ref: '../components/schemas/4xx_response.yaml' servers: - - url: https://localhost:5601 + - url: https://localhost:5601 delete: summary: Deletes one or more cases. @@ -103,7 +103,7 @@ patch: schema: $ref: '../components/schemas/4xx_response.yaml' servers: - - url: https://localhost:5601 + - url: https://localhost:5601 servers: - - url: https://localhost:5601 \ No newline at end of file + - url: https://localhost:5601 diff --git a/x-pack/plugins/cases/server/client/cases/delete.test.ts b/x-pack/plugins/cases/server/client/cases/delete.test.ts index ebe64da0c2baf..37be71049cf2d 100644 --- a/x-pack/plugins/cases/server/client/cases/delete.test.ts +++ b/x-pack/plugins/cases/server/client/cases/delete.test.ts @@ -4,7 +4,7 @@ * 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 { MAX_DELETE_IDS_LENGTH, 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'; @@ -95,6 +95,22 @@ describe('delete', () => { }); }); }); + + describe('errors', () => { + it(`throws 400 when trying to delete more than ${MAX_DELETE_IDS_LENGTH} cases at a time`, async () => { + const caseIds = new Array(MAX_DELETE_IDS_LENGTH + 1).fill('id'); + + await expect(deleteCases(caseIds, clientArgs)).rejects.toThrowError( + 'Error: The length of the field ids is too long. Array must be of length <= 100.' + ); + }); + + it('throws 400 when no id is passed to delete', async () => { + await expect(deleteCases([], clientArgs)).rejects.toThrowError( + 'Error: The length of the field ids is too short. Array must be of length >= 1.' + ); + }); + }); }); }); diff --git a/x-pack/plugins/cases/server/client/cases/delete.ts b/x-pack/plugins/cases/server/client/cases/delete.ts index 5fbefa1b4405b..78fffc19a2edc 100644 --- a/x-pack/plugins/cases/server/client/cases/delete.ts +++ b/x-pack/plugins/cases/server/client/cases/delete.ts @@ -10,6 +10,8 @@ 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 type { CasesDeleteRequest } from '../../../common/api'; +import { CasesDeleteRequestRt, decodeWithExcessOrThrow } from '../../../common/api'; import { CASE_COMMENT_SAVED_OBJECT, CASE_SAVED_OBJECT, @@ -26,7 +28,10 @@ import { createFileEntities, deleteFiles } from '../files'; /** * Deletes the specified cases and their attachments. */ -export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): Promise { +export async function deleteCases( + ids: CasesDeleteRequest, + clientArgs: CasesClientArgs +): Promise { const { services: { caseService, attachmentService, userActionService, alertsService }, logger, @@ -35,7 +40,8 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P } = clientArgs; try { - const cases = await caseService.getCases({ caseIds: ids }); + const caseIds = decodeWithExcessOrThrow(CasesDeleteRequestRt)(ids); + const cases = await caseService.getCases({ caseIds }); const entities = new Map(); for (const theCase of cases.saved_objects) { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts index 6da8cfccf1352..5166f7b135380 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts @@ -143,6 +143,22 @@ export default ({ getService }: FtrProviderContext): void => { await deleteCases({ supertest, caseIDs: ['fake-id'], expectedHttpCode: 404 }); }); + it('unhappy path - 400s when trying to delete more than 100 cases at a time', async () => { + await deleteCases({ + supertest: supertestWithoutAuth, + caseIDs: new Array(101).fill('id'), + expectedHttpCode: 400, + }); + }); + + it('unhappy path - 400s when trying to delete 0 cases at a time', async () => { + await deleteCases({ + supertest: supertestWithoutAuth, + caseIDs: [], + expectedHttpCode: 400, + }); + }); + describe('files', () => { afterEach(async () => { await deleteAllFiles({