From 429dd716ba6449ee90666ae6cd974158ccd5558c Mon Sep 17 00:00:00 2001 From: adcoelho Date: Thu, 29 Jun 2023 10:52:04 +0200 Subject: [PATCH 1/2] Limit number of case ids in delete cases api. Tests. Documentation --- x-pack/plugins/cases/common/api/cases/case.ts | 10 ++++++++++ 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/paths/s@{spaceid}@api@cases.yaml | 14 +++++++++----- .../cases/server/client/cases/delete.test.ts | 18 +++++++++++++++++- .../cases/server/client/cases/delete.ts | 10 ++++++++-- .../tests/common/cases/delete_cases.ts | 16 ++++++++++++++++ 8 files changed, 72 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index 8f1e7a1264166..be4c619292757 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -13,6 +13,8 @@ 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 } from '../../constants'; export const AttachmentTotalsRt = rt.strict({ alerts: rt.number, @@ -283,6 +285,13 @@ export const CasesFindRequestRt = rt.exact( }) ); +export const CasesDeleteRequestRt = limitedArraySchema( + NonEmptyString, + 1, + MAX_DELETE_IDS_LENGTH, + 'ids' +); + export const CasesByAlertIDRequestRt = rt.exact( rt.partial({ /** @@ -420,6 +429,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 1a49de004e73f..4639f59f63f0b 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -106,6 +106,7 @@ export const MAX_CONCURRENT_SEARCHES = 10 as const; export const MAX_BULK_GET_CASES = 1000 as const; export const MAX_COMMENTS_PER_PAGE = 100 as const; export const MAX_CATEGORY_FILTER_LENGTH = 100 as const; +export const MAX_DELETE_IDS_LENGTH = 100 as const; /** * Validation diff --git a/x-pack/plugins/cases/docs/openapi/bundled.json b/x-pack/plugins/cases/docs/openapi/bundled.json index 78ba5444f40aa..f6706d741e7ff 100644 --- a/x-pack/plugins/cases/docs/openapi/bundled.json +++ b/x-pack/plugins/cases/docs/openapi/bundled.json @@ -109,7 +109,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 405cf4fb689f0..b2b95f218636a 100644 --- a/x-pack/plugins/cases/docs/openapi/bundled.yaml +++ b/x-pack/plugins/cases/docs/openapi/bundled.yaml @@ -68,7 +68,11 @@ paths: in: query required: true schema: - type: string + type: array + items: + type: string + minItems: 1 + maxItems: 100 example: d4e7abb0-b462-11ec-9a8d-698504725a43 responses: '204': 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 ba4b852d96405..985d3a888cd7e 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. @@ -56,7 +56,11 @@ delete: in: query required: true schema: - type: string + type: array + items: + type: string + minItems: 1 + maxItems: 100 example: d4e7abb0-b462-11ec-9a8d-698504725a43 responses: '204': @@ -109,7 +113,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..684b234cae833 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} files 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({ From 6d346700db73d6a652ee60f1b4d8581436c05bd0 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Thu, 29 Jun 2023 18:04:50 +0200 Subject: [PATCH 2/2] Addressing pr comments. --- x-pack/plugins/cases/common/constants/index.ts | 2 +- x-pack/plugins/cases/server/client/cases/delete.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts index 4639f59f63f0b..cd330da4a5690 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -106,7 +106,6 @@ export const MAX_CONCURRENT_SEARCHES = 10 as const; export const MAX_BULK_GET_CASES = 1000 as const; export const MAX_COMMENTS_PER_PAGE = 100 as const; export const MAX_CATEGORY_FILTER_LENGTH = 100 as const; -export const MAX_DELETE_IDS_LENGTH = 100 as const; /** * Validation @@ -114,6 +113,7 @@ export const MAX_DELETE_IDS_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/server/client/cases/delete.test.ts b/x-pack/plugins/cases/server/client/cases/delete.test.ts index 684b234cae833..37be71049cf2d 100644 --- a/x-pack/plugins/cases/server/client/cases/delete.test.ts +++ b/x-pack/plugins/cases/server/client/cases/delete.test.ts @@ -97,7 +97,7 @@ describe('delete', () => { }); describe('errors', () => { - it(`throws 400 when trying to delete more than ${MAX_DELETE_IDS_LENGTH} files at a time`, async () => { + 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(