diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts index a4bd4b016d955..9778195465432 100644 --- a/x-pack/plugins/cases/common/api/cases/case.ts +++ b/x-pack/plugins/cases/common/api/cases/case.ts @@ -24,6 +24,7 @@ import { MAX_ASSIGNEES_FILTER_LENGTH, MAX_REPORTERS_FILTER_LENGTH, MAX_TAGS_FILTER_LENGTH, + MAX_BULK_GET_CASES, } from '../../constants'; export const AttachmentTotalsRt = rt.strict({ @@ -509,7 +510,7 @@ export const GetCategoriesResponseRt = rt.array(rt.string); export const GetReportersResponseRt = rt.array(UserRt); export const CasesBulkGetRequestRt = rt.strict({ - ids: rt.array(rt.string), + ids: limitedArraySchema({ codec: rt.string, min: 1, max: MAX_BULK_GET_CASES, fieldName: 'ids' }), }); export const CasesBulkGetResponseRt = rt.strict({ diff --git a/x-pack/plugins/cases/common/api/cases/comment/index.ts b/x-pack/plugins/cases/common/api/cases/comment/index.ts index f688dc24fdf85..1662631ae2b79 100644 --- a/x-pack/plugins/cases/common/api/cases/comment/index.ts +++ b/x-pack/plugins/cases/common/api/cases/comment/index.ts @@ -6,6 +6,8 @@ */ import * as rt from 'io-ts'; +import { MAX_BULK_GET_ATTACHMENTS } from '../../../constants'; +import { limitedArraySchema } from '../../../schema'; import { jsonValueRt } from '../../runtime_types'; import { NumberFromString } from '../../saved_object'; @@ -307,7 +309,12 @@ export const FindCommentsQueryParamsRt = rt.exact( export const BulkCreateCommentRequestRt = rt.array(CommentRequestRt); export const BulkGetAttachmentsRequestRt = rt.strict({ - ids: rt.array(rt.string), + ids: limitedArraySchema({ + codec: rt.string, + min: 1, + max: MAX_BULK_GET_ATTACHMENTS, + fieldName: 'ids', + }), }); export const BulkGetAttachmentsResponseRt = rt.strict({ diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts index bee8ff9f3b17e..b7b4987042495 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -101,7 +101,7 @@ export const MAX_ALERTS_PER_CASE = 1000 as const; * Searching */ export const MAX_DOCS_PER_PAGE = 10000 as const; -export const MAX_BULK_GET_ATTACHMENTS = MAX_DOCS_PER_PAGE; +export const MAX_BULK_GET_ATTACHMENTS = 100 as const; 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; diff --git a/x-pack/plugins/cases/server/client/attachments/bulk_get.test.ts b/x-pack/plugins/cases/server/client/attachments/bulk_get.test.ts new file mode 100644 index 0000000000000..078f79128391b --- /dev/null +++ b/x-pack/plugins/cases/server/client/attachments/bulk_get.test.ts @@ -0,0 +1,41 @@ +/* + * 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_BULK_GET_ATTACHMENTS } from '../../../common/constants'; +import { createCasesClientMockArgs, createCasesClientMock } from '../mocks'; +import { bulkGet } from './bulk_get'; + +describe('bulkGet', () => { + describe('errors', () => { + const casesClient = createCasesClientMock(); + const clientArgs = createCasesClientMockArgs(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it(`throws when trying to fetch more than ${MAX_BULK_GET_ATTACHMENTS} attachments`, async () => { + await expect( + bulkGet( + { attachmentIDs: Array(MAX_BULK_GET_ATTACHMENTS + 1).fill('foobar'), caseID: '123' }, + clientArgs, + casesClient + ) + ).rejects.toThrow( + `Error: The length of the field ids is too long. Array must be of length <= ${MAX_BULK_GET_ATTACHMENTS}.` + ); + }); + + it('throws when trying to fetch zero attachments', async () => { + await expect( + bulkGet({ attachmentIDs: [], caseID: '123' }, clientArgs, casesClient) + ).rejects.toThrow( + '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/attachments/bulk_get.ts b/x-pack/plugins/cases/server/client/attachments/bulk_get.ts index 294b39bab6959..553f8ca258669 100644 --- a/x-pack/plugins/cases/server/client/attachments/bulk_get.ts +++ b/x-pack/plugins/cases/server/client/attachments/bulk_get.ts @@ -5,10 +5,7 @@ * 2.0. */ -import Boom from '@hapi/boom'; - import { partition } from 'lodash'; -import { MAX_BULK_GET_ATTACHMENTS } from '../../../common/constants'; import type { BulkGetAttachmentsResponse, CommentAttributes } from '../../../common/api'; import { decodeWithExcessOrThrow, @@ -45,8 +42,6 @@ export async function bulkGet( try { const request = decodeWithExcessOrThrow(BulkGetAttachmentsRequestRt)({ ids: attachmentIDs }); - throwErrorIfIdsExceedTheLimit(request.ids); - // perform an authorization check for the case await casesClient.cases.resolve({ id: caseID }); @@ -83,14 +78,6 @@ export async function bulkGet( } } -const throwErrorIfIdsExceedTheLimit = (ids: string[]) => { - if (ids.length > MAX_BULK_GET_ATTACHMENTS) { - throw Boom.badRequest( - `Maximum request limit of ${MAX_BULK_GET_ATTACHMENTS} attachments reached` - ); - } -}; - interface PartitionedAttachments { validAttachments: AttachmentSavedObject[]; attachmentsWithErrors: AttachmentSavedObjectWithErrors; diff --git a/x-pack/plugins/cases/server/client/cases/bulk_get.test.ts b/x-pack/plugins/cases/server/client/cases/bulk_get.test.ts index 0ee8955c9adb5..771f5ece6f188 100644 --- a/x-pack/plugins/cases/server/client/cases/bulk_get.test.ts +++ b/x-pack/plugins/cases/server/client/cases/bulk_get.test.ts @@ -5,22 +5,29 @@ * 2.0. */ +import { MAX_BULK_GET_CASES } from '../../../common/constants'; import { createCasesClientMockArgs } from '../mocks'; import { bulkGet } from './bulk_get'; describe('bulkGet', () => { - describe('throwErrorIfCaseIdsReachTheLimit', () => { + describe('errors', () => { const clientArgs = createCasesClientMockArgs(); beforeEach(() => { jest.clearAllMocks(); }); - it('throws if the requested cases are more than 1000', async () => { - const ids = Array(1001).fill('test'); + it(`throws when trying to fetch more than ${MAX_BULK_GET_CASES} cases`, async () => { + await expect( + bulkGet({ ids: Array(MAX_BULK_GET_CASES + 1).fill('foobar') }, clientArgs) + ).rejects.toThrow( + `Error: The length of the field ids is too long. Array must be of length <= ${MAX_BULK_GET_CASES}.` + ); + }); - await expect(bulkGet({ ids }, clientArgs)).rejects.toThrow( - 'Maximum request limit of 1000 cases reached' + it('throws when trying to fetch zero cases', async () => { + await expect(bulkGet({ ids: [] }, clientArgs)).rejects.toThrow( + '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/bulk_get.ts b/x-pack/plugins/cases/server/client/cases/bulk_get.ts index 13df81bf06965..4665c27cebbf6 100644 --- a/x-pack/plugins/cases/server/client/cases/bulk_get.ts +++ b/x-pack/plugins/cases/server/client/cases/bulk_get.ts @@ -5,10 +5,8 @@ * 2.0. */ -import Boom from '@hapi/boom'; import { partition } from 'lodash'; -import { MAX_BULK_GET_CASES } from '../../../common/constants'; import type { CasesBulkGetResponse, CasesBulkGetRequest, @@ -45,8 +43,6 @@ export const bulkGet = async ( try { const request = decodeWithExcessOrThrow(CasesBulkGetRequestRt)(params); - throwErrorIfCaseIdsReachTheLimit(request.ids); - const cases = await caseService.getCases({ caseIds: request.ids }); const [validCases, soBulkGetErrors] = partition( @@ -91,12 +87,6 @@ export const bulkGet = async ( } }; -const throwErrorIfCaseIdsReachTheLimit = (ids: string[]) => { - if (ids.length > MAX_BULK_GET_CASES) { - throw Boom.badRequest(`Maximum request limit of ${MAX_BULK_GET_CASES} cases reached`); - } -}; - const constructErrors = ( soBulkGetErrors: CaseSavedObjectWithErrors, unauthorizedCases: CaseSavedObjectTransformed[] diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_attachments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_attachments.ts index 2d1a7c2ab69a6..8907f7e28841c 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_attachments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_attachments.ts @@ -90,27 +90,6 @@ export default ({ getService }: FtrProviderContext): void => { expect(response.attachments[1].id).to.eql(updatedCase.comments![1].id); }); - it('returns an empty array when no ids are requested', async () => { - const { attachments, errors } = await bulkGetAttachments({ - attachmentIds: [], - caseId: updatedCase.id, - supertest, - expectedHttpCode: 200, - }); - - expect(attachments.length).to.be(0); - expect(errors.length).to.be(0); - }); - - it('returns a 400 when more than 10k ids are requested', async () => { - await bulkGetAttachments({ - attachmentIds: Array.from(Array(10001).keys()).map((item) => item.toString()), - caseId: updatedCase.id, - supertest, - expectedHttpCode: 400, - }); - }); - it('populates the errors field with attachments that could not be found', async () => { const response = await bulkGetAttachments({ attachmentIds: [updatedCase.comments![0].id, 'does-not-exist'], @@ -455,5 +434,25 @@ export default ({ getService }: FtrProviderContext): void => { }); } }); + + describe('errors', () => { + it('400s when requesting more than 100 attachments', async () => { + await bulkGetAttachments({ + attachmentIds: Array(101).fill('foobar'), + caseId: 'id', + expectedHttpCode: 400, + supertest, + }); + }); + + it('400s when requesting zero attachments', async () => { + await bulkGetAttachments({ + attachmentIds: [], + caseId: 'id', + expectedHttpCode: 400, + supertest, + }); + }); + }); }); }; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts index 3ea30d7371b3e..449251377a077 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_get_cases.ts @@ -7,6 +7,7 @@ import expect from '@kbn/expect'; import { CommentType } from '@kbn/cases-plugin/common'; +import { MAX_BULK_GET_CASES } from '@kbn/cases-plugin/common/constants'; import { getPostCaseRequest, postCaseReq } from '../../../../common/lib/mock'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -111,12 +112,18 @@ export default ({ getService }: FtrProviderContext): void => { }); describe('errors', () => { - it('400s when requesting more than 1000 cases', async () => { - const ids = Array(1001).fill('test'); + it(`400s when requesting more than ${MAX_BULK_GET_CASES} cases`, async () => { + await bulkGetCases({ + supertest, + ids: Array(MAX_BULK_GET_CASES + 1).fill('foobar'), + expectedHttpCode: 400, + }); + }); + it('400s when requesting zero cases', async () => { await bulkGetCases({ supertest, - ids, + ids: [], expectedHttpCode: 400, }); });