[Cases] Guardrails: Limit bulk get cases and bulk get attachments (#1…

…61088)

Connected to #146945

## Summary

| Description  | Limit | Done? | Documented?
| ------------- | ---- | :---: | ---- |
| Total number of attachments returned by bulk get API | 100 |
✅ | No (internal) |
| Total number of cases returned by bulk get API | 1000 |
✅ | No (internal) |

- Replaced the code validation with schema validation.
- BulkGet Cases
- The minimum of cases that can be returned is now 1(was not validated
before).
- BulkGet Attachments
- The maximum of attachments that can be returned is now 100(was
**10000**).
- The minimum of attachments that can be returned is now 1(was not
validated before).
- Updated unit and e2e tests.
- The documentation was not updated because these are internal APIs.
- Skipping the release notes because these are internal APIs.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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

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({

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({

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;

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.'
+      );
+    });
+  });
+});

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;

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.'
       );
     });
 

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[]

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,
+        });
+      });
+    });
   });
 };

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,
         });
       });