From e6cfc4d27389825e0556575cf58e0e2c288c54e4 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 17 Jul 2023 17:41:08 +0200 Subject: [PATCH 1/6] Validate persistableState and externalReferences. --- .../plugins/cases/common/constants/index.ts | 1 + .../common/limiter_checker/base_limiter.ts | 4 +- .../server/common/limiter_checker/index.ts | 7 ++- .../persistableStateAndExternalReferences.ts | 37 ++++++++++++++ x-pack/plugins/cases/server/common/utils.ts | 10 ++++ .../server/services/attachments/index.ts | 51 ++++++++++++++++++- 6 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.ts diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts index 33c0f14a81501..be79848b91057 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -127,6 +127,7 @@ export const MAX_DELETE_IDS_LENGTH = 100 as const; export const MAX_SUGGESTED_PROFILES = 10 as const; export const MAX_CASES_TO_UPDATE = 100 as const; export const MAX_BULK_CREATE_ATTACHMENTS = 100 as const; +export const MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES = 100 as const; /** * Cases features diff --git a/x-pack/plugins/cases/server/common/limiter_checker/base_limiter.ts b/x-pack/plugins/cases/server/common/limiter_checker/base_limiter.ts index cf9fcec712e61..f4e72554636ae 100644 --- a/x-pack/plugins/cases/server/common/limiter_checker/base_limiter.ts +++ b/x-pack/plugins/cases/server/common/limiter_checker/base_limiter.ts @@ -10,8 +10,8 @@ import type { Limiter } from './types'; interface LimiterParams { limit: number; - attachmentType: CommentType; - field: string; + attachmentType: CommentType | CommentType[]; + field?: string; attachmentNoun: string; } diff --git a/x-pack/plugins/cases/server/common/limiter_checker/index.ts b/x-pack/plugins/cases/server/common/limiter_checker/index.ts index 97928f2eeb356..31a78cc7526d9 100644 --- a/x-pack/plugins/cases/server/common/limiter_checker/index.ts +++ b/x-pack/plugins/cases/server/common/limiter_checker/index.ts @@ -13,6 +13,7 @@ import type { AttachmentService } from '../../services'; import type { Limiter } from './types'; import { AlertLimiter } from './limiters/alerts'; import { FileLimiter } from './limiters/files'; +import { PersistableStateAndExternalReferencesLimiter } from './limiters/persistableStateAndExternalReferences'; export class AttachmentLimitChecker { private readonly limiters: Limiter[]; @@ -22,7 +23,11 @@ export class AttachmentLimitChecker { fileService: FileServiceStart, private readonly caseId: string ) { - this.limiters = [new AlertLimiter(attachmentService), new FileLimiter(fileService)]; + this.limiters = [ + new AlertLimiter(attachmentService), + new FileLimiter(fileService), + new PersistableStateAndExternalReferencesLimiter(attachmentService), + ]; } public async validate(requests: CommentRequest[]) { diff --git a/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.ts b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.ts new file mode 100644 index 0000000000000..b16baa919cffd --- /dev/null +++ b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.ts @@ -0,0 +1,37 @@ +/* + * 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 type { AttachmentService } from '../../../services'; +import { CommentType } from '../../../../common/api'; +import type { CommentRequest } from '../../../../common/api'; +import { MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES } from '../../../../common/constants'; +import { isFileAttachmentRequest, isPersistableStateOrExternalReference } from '../../utils'; +import { BaseLimiter } from '../base_limiter'; + +export class PersistableStateAndExternalReferencesLimiter extends BaseLimiter { + constructor(private readonly attachmentService: AttachmentService) { + super({ + limit: MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES, + attachmentType: [CommentType.persistableState, CommentType.externalReference], + attachmentNoun: 'persistable state and external reference attachments', + }); + } + + public async countOfItemsWithinCase(caseId: string): Promise { + return this.attachmentService.countPersistableStateAndExternalReferenceAttachments({ + caseId, + }); + } + + public countOfItemsInRequest(requests: CommentRequest[]): number { + const totalReferences = requests + .filter(isPersistableStateOrExternalReference) + .filter((request) => !isFileAttachmentRequest(request)); + + return totalReferences.length; + } +} diff --git a/x-pack/plugins/cases/server/common/utils.ts b/x-pack/plugins/cases/server/common/utils.ts index 7b8164133f5a3..96d84d8a91442 100644 --- a/x-pack/plugins/cases/server/common/utils.ts +++ b/x-pack/plugins/cases/server/common/utils.ts @@ -254,6 +254,16 @@ export const isCommentRequestTypeAlert = ( return context.type === CommentType.alert; }; +/** + * Returns true if a Comment Request is trying to create either a persistableState or an + * externalReference attachment. + */ +export const isPersistableStateOrExternalReference = (context: CommentRequest): boolean => { + return ( + context.type === CommentType.persistableState || context.type === CommentType.externalReference + ); +}; + /** * A type narrowing function for file attachments. */ diff --git a/x-pack/plugins/cases/server/services/attachments/index.ts b/x-pack/plugins/cases/server/services/attachments/index.ts index b0d282e00aa96..8f5c9ee69c07c 100644 --- a/x-pack/plugins/cases/server/services/attachments/index.ts +++ b/x-pack/plugins/cases/server/services/attachments/index.ts @@ -14,8 +14,13 @@ import type { } from '@kbn/core/server'; import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { fromKueryExpression } from '@kbn/es-query'; import { CommentAttributesRt, CommentType, decodeOrThrow } from '../../../common/api'; -import { CASE_COMMENT_SAVED_OBJECT, CASE_SAVED_OBJECT } from '../../../common/constants'; +import { + CASE_COMMENT_SAVED_OBJECT, + CASE_SAVED_OBJECT, + FILE_ATTACHMENT_TYPE, +} from '../../../common/constants'; import { buildFilter, combineFilters } from '../../client/utils'; import { defaultSortField, isSOError } from '../../common/utils'; import type { AggregationResponse } from '../../client/metrics/types'; @@ -124,6 +129,50 @@ export class AttachmentService { } } + /** + * Counts the persistableState and externalReference attachments that are not .files + */ + public async countPersistableStateAndExternalReferenceAttachments({ + caseId, + }: { + caseId: string; + }): Promise { + try { + this.context.log.debug( + `Attempting to count persistableState and externalReference attachments for case id ${caseId}` + ); + + const typeFilter = buildFilter({ + filters: [CommentType.persistableState, CommentType.externalReference], + field: 'type', + operator: 'or', + type: CASE_COMMENT_SAVED_OBJECT, + }); + + const excludeFilesFilter = fromKueryExpression( + `not ${CASE_COMMENT_SAVED_OBJECT}.attributes.externalReferenceAttachmentTypeId: ${FILE_ATTACHMENT_TYPE}` + ); + + const combinedFilter = combineFilters([typeFilter, excludeFilesFilter]); + + const response = await this.context.unsecuredSavedObjectsClient.find<{ total: number }>({ + type: CASE_COMMENT_SAVED_OBJECT, + hasReference: { type: CASE_SAVED_OBJECT, id: caseId }, + page: 1, + perPage: 1, + sortField: defaultSortField, + filter: combinedFilter, + }); + + return response.total; + } catch (error) { + this.context.log.error( + `Error while attempting to count persistableState and externalReference attachments for case id ${caseId}: ${error}` + ); + throw error; + } + } + /** * Executes the aggregations against the actions attached to a case. */ From e33fdd972240fc73d471649501afa0d943446f22 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Thu, 20 Jul 2023 17:31:39 +0200 Subject: [PATCH 2/6] Tests. --- ...sistableStateAndExternalReferences.test.ts | 84 +++++++++++++++++++ .../common/limiter_checker/test_utils.ts | 33 ++++++++ .../plugins/cases/server/common/utils.test.ts | 28 +++++++ x-pack/plugins/cases/server/services/mocks.ts | 1 + 4 files changed, 146 insertions(+) create mode 100644 x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts diff --git a/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts new file mode 100644 index 0000000000000..8f64ff87564ae --- /dev/null +++ b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts @@ -0,0 +1,84 @@ +/* + * 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 { createAttachmentServiceMock } from '../../../services/mocks'; +import { PersistableStateAndExternalReferencesLimiter } from './persistableStateAndExternalReferences'; +import { + createExternalReferenceRequests, + createFileRequests, + createPersistableStateRequests, + createUserRequests, +} from '../test_utils'; +import { MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES } from '../../../../common/constants'; + +describe('PersistableStateAndExternalReferencesLimiter', () => { + const caseId = 'test-id'; + const attachmentService = createAttachmentServiceMock(); + attachmentService.countPersistableStateAndExternalReferenceAttachments.mockImplementation( + async () => { + return 1; + } + ); + + const limiter = new PersistableStateAndExternalReferencesLimiter(attachmentService); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('public fields', () => { + it('sets the errorMessage to the 1k limit', () => { + expect(limiter.errorMessage).toMatchInlineSnapshot( + `"Case has reached the maximum allowed number (100) of attached persistable state and external reference attachments."` + ); + }); + + it('sets the limit to 1k', () => { + expect(limiter.limit).toBe(MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES); + }); + }); + + describe('countOfItemsWithinCase', () => { + it('calls the attachment service with the right params', () => { + limiter.countOfItemsWithinCase(caseId); + + expect( + attachmentService.countPersistableStateAndExternalReferenceAttachments + ).toHaveBeenCalledWith({ caseId }); + }); + }); + + describe('countOfItemsInRequest', () => { + it('returns 0 when passed an empty array', () => { + expect(limiter.countOfItemsInRequest([])).toBe(0); + }); + + it('returns 0 when the requests are not for persistable state attachments or external references', () => { + expect(limiter.countOfItemsInRequest(createUserRequests(2))).toBe(0); + }); + + it('counts persistable state attachments or external references correctly', () => { + expect( + limiter.countOfItemsInRequest([ + createPersistableStateRequests(1)[0], + createExternalReferenceRequests(1)[0], + ]) + ).toBe(2); + }); + + it('excludes fileAttachmentsRequests from the count', () => { + expect( + limiter.countOfItemsInRequest( + createFileRequests({ + numRequests: 1, + numFiles: 1, + }) + ) + ).toBe(0); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts b/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts index f0c990e439bdf..bddb686609044 100644 --- a/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts +++ b/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts @@ -11,6 +11,8 @@ import type { CommentRequestUserType, CommentRequestAlertType, FileAttachmentMetadata, + CommentRequestPersistableStateType, + CommentRequestExternalReferenceType, } from '../../../common/api'; import type { FileAttachmentRequest } from '../types'; @@ -26,6 +28,37 @@ export const createUserRequests = (num: number): CommentRequestUserType[] => { return requests; }; +export const createPersistableStateRequests = ( + num: number +): CommentRequestPersistableStateType[] => { + return [...Array(num).keys()].map((value) => { + return { + persistableStateAttachmentTypeId: 'some-id', + persistableStateAttachmentState: {}, + type: CommentType.persistableState as const, + owner: 'test', + }; + }); +}; + +export const createExternalReferenceRequests = ( + num: number +): CommentRequestExternalReferenceType[] => { + return [...Array(num).keys()].map((value) => { + return { + type: CommentType.externalReference as const, + owner: 'test', + externalReferenceAttachmentTypeId: 'doesnt-matter', + externalReferenceId: 'so-id', + externalReferenceMetadata: {}, + externalReferenceStorage: { + soType: `${value}`, + type: ExternalReferenceStorageType.savedObject, + }, + }; + }); +}; + export const createFileRequests = ({ numRequests, numFiles, diff --git a/x-pack/plugins/cases/server/common/utils.test.ts b/x-pack/plugins/cases/server/common/utils.test.ts index 1c42d75be05d1..ce470b66080ca 100644 --- a/x-pack/plugins/cases/server/common/utils.test.ts +++ b/x-pack/plugins/cases/server/common/utils.test.ts @@ -33,6 +33,7 @@ import { getCaseViewPath, isSOError, countUserAttachments, + isPersistableStateOrExternalReference, } from './utils'; import { newCase } from '../routes/api/__mocks__/request_responses'; import { CASE_VIEW_PAGE_TABS } from '../../common/types'; @@ -40,6 +41,12 @@ import { mockCases, mockCaseComments } from '../mocks'; import { createAlertAttachment, createUserAttachment } from '../services/attachments/test_utils'; import type { CaseConnector } from '../../common/types/domain'; import { ConnectorTypes } from '../../common/types/domain'; +import { + createAlertRequests, + createExternalReferenceRequests, + createPersistableStateRequests, + createUserRequests, +} from './limiter_checker/test_utils'; interface CommentReference { ids: string[]; @@ -1353,4 +1360,25 @@ describe('common utils', () => { expect(countUserAttachments(attachments)).toBe(0); }); }); + + describe('isPersistableStateOrExternalReference', () => { + it('returns true for persistable state request', () => { + expect(isPersistableStateOrExternalReference(createPersistableStateRequests(1)[0])).toBe( + true + ); + }); + + it('returns true for external reference request', () => { + expect(isPersistableStateOrExternalReference(createExternalReferenceRequests(1)[0])).toBe( + true + ); + }); + + it('returns false for other request types', () => { + expect(isPersistableStateOrExternalReference(createUserRequests(1)[0])).toBe(false); + expect(isPersistableStateOrExternalReference(createAlertRequests(1, 'alert-id')[0])).toBe( + false + ); + }); + }); }); diff --git a/x-pack/plugins/cases/server/services/mocks.ts b/x-pack/plugins/cases/server/services/mocks.ts index e9cb492e39458..b43a3c226b1e4 100644 --- a/x-pack/plugins/cases/server/services/mocks.ts +++ b/x-pack/plugins/cases/server/services/mocks.ts @@ -177,6 +177,7 @@ export const createAttachmentServiceMock = (): AttachmentServiceMock => { countAlertsAttachedToCase: jest.fn(), executeCaseActionsAggregations: jest.fn(), executeCaseAggregations: jest.fn(), + countPersistableStateAndExternalReferenceAttachments: jest.fn(), }; // the cast here is required because jest.Mocked tries to include private members and would throw an error From c0e0d82db9ec6871492901233553327281d7ceea Mon Sep 17 00:00:00 2001 From: adcoelho Date: Thu, 20 Jul 2023 18:08:23 +0200 Subject: [PATCH 3/6] Final tests. --- .../plugins/cases/server/client/cases/mock.ts | 12 ++- .../common/models/case_with_comments.test.ts | 92 +++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/server/client/cases/mock.ts b/x-pack/plugins/cases/server/client/cases/mock.ts index bee726524061a..7e92e8c2e1beb 100644 --- a/x-pack/plugins/cases/server/client/cases/mock.ts +++ b/x-pack/plugins/cases/server/client/cases/mock.ts @@ -9,7 +9,7 @@ import type { CaseUserActionsDeprecatedResponse } from '../../../common/types/ap import { ConnectorTypes, UserActionActions } from '../../../common/types/domain'; import type { Comment, CommentResponseAlertsType } from '../../../common/api'; import { CommentType, ExternalReferenceStorageType } from '../../../common/api'; -import { SECURITY_SOLUTION_OWNER } from '../../../common/constants'; +import { FILE_ATTACHMENT_TYPE, SECURITY_SOLUTION_OWNER } from '../../../common/constants'; export const updateUser = { updated_at: '2020-03-13T08:34:53.450Z', @@ -228,6 +228,16 @@ export const commentPersistableState: Comment = { version: 'WzEsMV0=', }; +export const commentFilePersistableState: Comment = { + ...commentExternalReference, + externalReferenceAttachmentTypeId: FILE_ATTACHMENT_TYPE, + externalReferenceMetadata: { files: [{ name: '', extension: '', mimeType: '', created: '' }] }, + externalReferenceStorage: { + type: ExternalReferenceStorageType.savedObject as const, + soType: 'iuhu', + }, +}; + export const basicParams = { description: 'a description', title: 'a title', diff --git a/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts b/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts index 94b3ac3e3be3d..f5684969797d4 100644 --- a/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts +++ b/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts @@ -10,6 +10,12 @@ import type { SavedObject } from '@kbn/core-saved-objects-api-server'; import { createCasesClientMockArgs } from '../../client/mocks'; import { alertComment, comment, mockCaseComments, mockCases, multipleAlert } from '../../mocks'; import { CaseCommentModel } from './case_with_comments'; +import { MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES } from '../../../common/constants'; +import { + commentExternalReference, + commentFilePersistableState, + commentPersistableState, +} from '../../client/cases/mock'; describe('CaseCommentModel', () => { const theCase = mockCases[0]; @@ -267,6 +273,52 @@ describe('CaseCommentModel', () => { expect(clientArgs.services.attachmentService.create).not.toHaveBeenCalled(); }); + + describe('validation', () => { + clientArgs.services.attachmentService.countPersistableStateAndExternalReferenceAttachments.mockResolvedValue( + MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES + ); + + afterAll(() => { + jest.clearAllMocks(); + }); + + it('throws if limit is reached when creating persistable state attachment', async () => { + await expect( + model.createComment({ + id: 'comment-1', + commentReq: commentPersistableState, + createdDate, + }) + ).rejects.toThrow( + `Case has reached the maximum allowed number (${MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES}) of attached persistable state and external reference attachments.` + ); + }); + + it('throws if limit is reached when creating external reference', async () => { + await expect( + model.createComment({ + id: 'comment-1', + commentReq: commentExternalReference, + createdDate, + }) + ).rejects.toThrow( + `Case has reached the maximum allowed number (${MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES}) of attached persistable state and external reference attachments.` + ); + }); + + it('does not throw if creating a file external reference and the limit is reached', async () => { + clientArgs.fileService.find.mockResolvedValue({ total: 0, files: [] }); + + await expect( + model.createComment({ + id: 'comment-1', + commentReq: commentFilePersistableState, + createdDate, + }) + ).resolves.not.toThrow(); + }); + }); }); describe('bulkCreate', () => { @@ -526,5 +578,45 @@ describe('CaseCommentModel', () => { expect(multipleAlertsCall.attributes.alertId).toEqual(['test-id-3', 'test-id-5']); expect(multipleAlertsCall.attributes.index).toEqual(['test-index-3', 'test-index-5']); }); + + describe('validation', () => { + clientArgs.services.attachmentService.countPersistableStateAndExternalReferenceAttachments.mockResolvedValue( + MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES + ); + + afterAll(() => { + jest.clearAllMocks(); + }); + + it('throws if limit is reached when creating persistable state attachment', async () => { + await expect( + model.bulkCreate({ + attachments: [commentPersistableState], + }) + ).rejects.toThrow( + `Case has reached the maximum allowed number (${MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES}) of attached persistable state and external reference attachments.` + ); + }); + + it('throws if limit is reached when creating external reference', async () => { + await expect( + model.bulkCreate({ + attachments: [commentExternalReference], + }) + ).rejects.toThrow( + `Case has reached the maximum allowed number (${MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES}) of attached persistable state and external reference attachments.` + ); + }); + + it('does not throw if creating a file external reference and the limit is reached', async () => { + clientArgs.fileService.find.mockResolvedValue({ total: 0, files: [] }); + + await expect( + model.bulkCreate({ + attachments: [commentFilePersistableState], + }) + ).resolves.not.toThrow(); + }); + }); }); }); From 3fa3b398ec8eb9bd13a450f559aec2f340d80b3f Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 24 Jul 2023 14:25:49 +0200 Subject: [PATCH 4/6] Addressing PR comments. --- .../plugins/cases/server/client/cases/mock.ts | 4 +-- ...ble_state_and_external_references.test.ts} | 17 ++++----- ...sistable_state_and_external_references.ts} | 0 .../common/models/case_with_comments.test.ts | 6 ++-- .../server/services/attachments/index.test.ts | 35 +++++++++++++++++++ 5 files changed, 49 insertions(+), 13 deletions(-) rename x-pack/plugins/cases/server/common/limiter_checker/limiters/{persistableStateAndExternalReferences.test.ts => persistable_state_and_external_references.test.ts} (89%) rename x-pack/plugins/cases/server/common/limiter_checker/limiters/{persistableStateAndExternalReferences.ts => persistable_state_and_external_references.ts} (100%) diff --git a/x-pack/plugins/cases/server/client/cases/mock.ts b/x-pack/plugins/cases/server/client/cases/mock.ts index 7e92e8c2e1beb..ca96d20bad570 100644 --- a/x-pack/plugins/cases/server/client/cases/mock.ts +++ b/x-pack/plugins/cases/server/client/cases/mock.ts @@ -228,13 +228,13 @@ export const commentPersistableState: Comment = { version: 'WzEsMV0=', }; -export const commentFilePersistableState: Comment = { +export const commentFileExternalReference: Comment = { ...commentExternalReference, externalReferenceAttachmentTypeId: FILE_ATTACHMENT_TYPE, externalReferenceMetadata: { files: [{ name: '', extension: '', mimeType: '', created: '' }] }, externalReferenceStorage: { type: ExternalReferenceStorageType.savedObject as const, - soType: 'iuhu', + soType: 'file', }, }; diff --git a/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistable_state_and_external_references.test.ts similarity index 89% rename from x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts rename to x-pack/plugins/cases/server/common/limiter_checker/limiters/persistable_state_and_external_references.test.ts index 8f64ff87564ae..df40e3841f1d7 100644 --- a/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.test.ts +++ b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistable_state_and_external_references.test.ts @@ -6,7 +6,7 @@ */ import { createAttachmentServiceMock } from '../../../services/mocks'; -import { PersistableStateAndExternalReferencesLimiter } from './persistableStateAndExternalReferences'; +import { PersistableStateAndExternalReferencesLimiter } from './persistable_state_and_external_references'; import { createExternalReferenceRequests, createFileRequests, @@ -18,11 +18,7 @@ import { MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES } from '../../../../commo describe('PersistableStateAndExternalReferencesLimiter', () => { const caseId = 'test-id'; const attachmentService = createAttachmentServiceMock(); - attachmentService.countPersistableStateAndExternalReferenceAttachments.mockImplementation( - async () => { - return 1; - } - ); + attachmentService.countPersistableStateAndExternalReferenceAttachments.mockResolvedValue(1); const limiter = new PersistableStateAndExternalReferencesLimiter(attachmentService); @@ -31,13 +27,13 @@ describe('PersistableStateAndExternalReferencesLimiter', () => { }); describe('public fields', () => { - it('sets the errorMessage to the 1k limit', () => { + it('sets the errorMessage to the 100 limit', () => { expect(limiter.errorMessage).toMatchInlineSnapshot( `"Case has reached the maximum allowed number (100) of attached persistable state and external reference attachments."` ); }); - it('sets the limit to 1k', () => { + it('sets the limit to 100', () => { expect(limiter.limit).toBe(MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES); }); }); @@ -66,6 +62,11 @@ describe('PersistableStateAndExternalReferencesLimiter', () => { limiter.countOfItemsInRequest([ createPersistableStateRequests(1)[0], createExternalReferenceRequests(1)[0], + createUserRequests(1)[0], + createFileRequests({ + numRequests: 1, + numFiles: 1, + })[0], ]) ).toBe(2); }); diff --git a/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.ts b/x-pack/plugins/cases/server/common/limiter_checker/limiters/persistable_state_and_external_references.ts similarity index 100% rename from x-pack/plugins/cases/server/common/limiter_checker/limiters/persistableStateAndExternalReferences.ts rename to x-pack/plugins/cases/server/common/limiter_checker/limiters/persistable_state_and_external_references.ts diff --git a/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts b/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts index f5684969797d4..779ed8767a9b3 100644 --- a/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts +++ b/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts @@ -13,7 +13,7 @@ import { CaseCommentModel } from './case_with_comments'; import { MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES } from '../../../common/constants'; import { commentExternalReference, - commentFilePersistableState, + commentFileExternalReference, commentPersistableState, } from '../../client/cases/mock'; @@ -313,7 +313,7 @@ describe('CaseCommentModel', () => { await expect( model.createComment({ id: 'comment-1', - commentReq: commentFilePersistableState, + commentReq: commentFileExternalReference, createdDate, }) ).resolves.not.toThrow(); @@ -613,7 +613,7 @@ describe('CaseCommentModel', () => { await expect( model.bulkCreate({ - attachments: [commentFilePersistableState], + attachments: [commentFileExternalReference], }) ).resolves.not.toThrow(); }); diff --git a/x-pack/plugins/cases/server/services/attachments/index.test.ts b/x-pack/plugins/cases/server/services/attachments/index.test.ts index 54a48f0a53c67..5967cb6fb2600 100644 --- a/x-pack/plugins/cases/server/services/attachments/index.test.ts +++ b/x-pack/plugins/cases/server/services/attachments/index.test.ts @@ -537,4 +537,39 @@ describe('AttachmentService', () => { }); }); }); + + describe('countPersistableStateAndExternalReferenceAttachments', () => { + it('does not throw and calls unsecuredSavedObjectsClient.find with the right parameters', async () => { + unsecuredSavedObjectsClient.find.mockResolvedValue( + createSOFindResponse([{ ...createUserAttachment(), score: 0 }]) + ); + + await expect( + service.countPersistableStateAndExternalReferenceAttachments({ caseId: 'test-id' }) + ).resolves.not.toThrow(); + + await expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledWith( + expect.objectContaining({ + hasReference: { id: 'test-id', type: 'cases' }, + type: 'cases-comments', + }) + ); + }); + + it('returns the expected total', async () => { + const total = 3; + + unsecuredSavedObjectsClient.find.mockResolvedValue( + createSOFindResponse( + Array(total).fill({ ...createUserAttachment({ foo: 'bar' }), score: 0 }) + ) + ); + + const res = await service.countPersistableStateAndExternalReferenceAttachments({ + caseId: 'test-id', + }); + + expect(res).toBe(total); + }); + }); }); From 69d437b38903a23958cdaf80ecf7e981626395d5 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 24 Jul 2023 15:11:19 +0200 Subject: [PATCH 5/6] Integration tests. --- .../server/common/limiter_checker/index.ts | 2 +- .../common/limiter_checker/test_utils.ts | 6 +- .../tests/common/comments/post_comment.ts | 72 ++++++ .../internal/bulk_create_attachments.ts | 242 ++++++++++++------ 4 files changed, 234 insertions(+), 88 deletions(-) diff --git a/x-pack/plugins/cases/server/common/limiter_checker/index.ts b/x-pack/plugins/cases/server/common/limiter_checker/index.ts index 31a78cc7526d9..1ef34d70e7e25 100644 --- a/x-pack/plugins/cases/server/common/limiter_checker/index.ts +++ b/x-pack/plugins/cases/server/common/limiter_checker/index.ts @@ -13,7 +13,7 @@ import type { AttachmentService } from '../../services'; import type { Limiter } from './types'; import { AlertLimiter } from './limiters/alerts'; import { FileLimiter } from './limiters/files'; -import { PersistableStateAndExternalReferencesLimiter } from './limiters/persistableStateAndExternalReferences'; +import { PersistableStateAndExternalReferencesLimiter } from './limiters/persistable_state_and_external_references'; export class AttachmentLimitChecker { private readonly limiters: Limiter[]; diff --git a/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts b/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts index bddb686609044..fc9515c133069 100644 --- a/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts +++ b/x-pack/plugins/cases/server/common/limiter_checker/test_utils.ts @@ -31,9 +31,9 @@ export const createUserRequests = (num: number): CommentRequestUserType[] => { export const createPersistableStateRequests = ( num: number ): CommentRequestPersistableStateType[] => { - return [...Array(num).keys()].map((value) => { + return [...Array(num).keys()].map(() => { return { - persistableStateAttachmentTypeId: 'some-id', + persistableStateAttachmentTypeId: '.test', persistableStateAttachmentState: {}, type: CommentType.persistableState as const, owner: 'test', @@ -48,7 +48,7 @@ export const createExternalReferenceRequests = ( return { type: CommentType.externalReference as const, owner: 'test', - externalReferenceAttachmentTypeId: 'doesnt-matter', + externalReferenceAttachmentTypeId: '.test', externalReferenceId: 'so-id', externalReferenceMetadata: {}, externalReferenceStorage: { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts index 8851a95c6ebc3..0bcde310a224b 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts @@ -16,6 +16,7 @@ import { CaseStatuses, CommentRequestExternalReferenceSOType, CommentRequestAlertType, + ExternalReferenceStorageType, } from '@kbn/cases-plugin/common/api'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -42,6 +43,7 @@ import { getCaseUserActions, removeServerGeneratedPropertiesFromUserAction, getAllComments, + bulkCreateAttachments, } from '../../../../common/lib/api'; import { createSignalsIndex, @@ -468,6 +470,76 @@ export default ({ getService }: FtrProviderContext): void => { expectedHttpCode: 400, }); }); + + it('400s when attempting to add a persistable state to a case that already has 100', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + const attachments = Array(100).fill({ + type: CommentType.externalReference as const, + owner: 'securitySolutionFixture', + externalReferenceAttachmentTypeId: '.test', + externalReferenceId: 'so-id', + externalReferenceMetadata: {}, + externalReferenceStorage: { + soType: 'external-ref', + type: ExternalReferenceStorageType.savedObject as const, + }, + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: attachments, + expectedHttpCode: 200, + }); + + await createComment({ + supertest, + caseId: postedCase.id, + params: { + persistableStateAttachmentTypeId: '.test', + persistableStateAttachmentState: {}, + type: CommentType.persistableState as const, + owner: 'securitySolutionFixture', + }, + expectedHttpCode: 400, + }); + }); + + it('400s when attempting to add an external reference to a case that already has 100', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + const attachments = Array(100).fill({ + persistableStateAttachmentTypeId: '.test', + persistableStateAttachmentState: {}, + type: CommentType.persistableState as const, + owner: 'securitySolutionFixture', + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: attachments, + expectedHttpCode: 200, + }); + + await createComment({ + supertest, + caseId: postedCase.id, + params: { + type: CommentType.externalReference as const, + owner: 'securitySolutionFixture', + externalReferenceAttachmentTypeId: '.test', + externalReferenceId: 'so-id', + externalReferenceMetadata: {}, + externalReferenceStorage: { + soType: 'external-ref', + type: ExternalReferenceStorageType.savedObject as const, + }, + }, + expectedHttpCode: 400, + }); + }); }); describe('alerts', () => { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts index a0b626482f473..e5eaedb6dae8c 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts @@ -15,6 +15,7 @@ import { CaseStatuses, CommentRequestExternalReferenceSOType, CommentType, + ExternalReferenceStorageType, } from '@kbn/cases-plugin/common/api'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -42,6 +43,7 @@ import { createAndUploadFile, deleteAllFiles, getAllComments, + createComment, } from '../../../../common/lib/api'; import { createSignalsIndex, @@ -619,102 +621,174 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAndBulkCreateAttachments({ supertest, expectedHttpCode: 400 }); }); - it('400s when attempting to add more than 1K alerts to a case', async () => { - const alerts = [...Array(1001).keys()].map((num) => `test-${num}`); - const postedCase = await createCase(supertest, postCaseReq); - await bulkCreateAttachments({ - supertest, - caseId: postedCase.id, - params: [ - { - ...postCommentAlertReq, - alertId: alerts, - index: alerts, - }, - ], - expectedHttpCode: 400, + describe('validation', () => { + it('400s when attempting to add more than 1K alerts to a case', async () => { + const alerts = [...Array(1001).keys()].map((num) => `test-${num}`); + const postedCase = await createCase(supertest, postCaseReq); + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [ + { + ...postCommentAlertReq, + alertId: alerts, + index: alerts, + }, + ], + expectedHttpCode: 400, + }); }); - }); - it('400s when attempting to add more than 1K alerts to a case in the same request', async () => { - const alerts = [...Array(1001).keys()].map((num) => `test-${num}`); - const postedCase = await createCase(supertest, postCaseReq); - await bulkCreateAttachments({ - supertest, - caseId: postedCase.id, - params: [ - { - ...postCommentAlertReq, - alertId: alerts.slice(0, 500), - index: alerts.slice(0, 500), - }, - { - ...postCommentAlertReq, - alertId: alerts.slice(500, alerts.length), - index: alerts.slice(500, alerts.length), - }, - postCommentAlertReq, - ], - expectedHttpCode: 400, + it('400s when attempting to add more than 1K alerts to a case in the same request', async () => { + const alerts = [...Array(1001).keys()].map((num) => `test-${num}`); + const postedCase = await createCase(supertest, postCaseReq); + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [ + { + ...postCommentAlertReq, + alertId: alerts.slice(0, 500), + index: alerts.slice(0, 500), + }, + { + ...postCommentAlertReq, + alertId: alerts.slice(500, alerts.length), + index: alerts.slice(500, alerts.length), + }, + postCommentAlertReq, + ], + expectedHttpCode: 400, + }); }); - }); - it('400s when attempting to add an alert to a case that already has 1K alerts', async () => { - const alerts = [...Array(1000).keys()].map((num) => `test-${num}`); - const postedCase = await createCase(supertest, postCaseReq); - await bulkCreateAttachments({ - supertest, - caseId: postedCase.id, - params: [ - { - ...postCommentAlertReq, - alertId: alerts, - index: alerts, - }, - ], + it('400s when attempting to add an alert to a case that already has 1K alerts', async () => { + const alerts = [...Array(1000).keys()].map((num) => `test-${num}`); + const postedCase = await createCase(supertest, postCaseReq); + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [ + { + ...postCommentAlertReq, + alertId: alerts, + index: alerts, + }, + ], + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [ + { + ...postCommentAlertReq, + alertId: 'test-id', + index: 'test-index', + }, + ], + expectedHttpCode: 400, + }); }); - await bulkCreateAttachments({ - supertest, - caseId: postedCase.id, - params: [ - { - ...postCommentAlertReq, - alertId: 'test-id', - index: 'test-index', - }, - ], - expectedHttpCode: 400, + it('400s when the case already has alerts and the sum of existing and new alerts exceed 1k', async () => { + const alerts = [...Array(1200).keys()].map((num) => `test-${num}`); + const postedCase = await createCase(supertest, postCaseReq); + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [ + { + ...postCommentAlertReq, + alertId: alerts.slice(0, 500), + index: alerts.slice(0, 500), + }, + ], + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [ + { + ...postCommentAlertReq, + alertId: alerts.slice(500), + index: alerts.slice(500), + }, + postCommentAlertReq, + ], + expectedHttpCode: 400, + }); }); - }); - it('400s when the case already has alerts and the sum of existing and new alerts exceed 1k', async () => { - const alerts = [...Array(1200).keys()].map((num) => `test-${num}`); - const postedCase = await createCase(supertest, postCaseReq); - await bulkCreateAttachments({ - supertest, - caseId: postedCase.id, - params: [ - { - ...postCommentAlertReq, - alertId: alerts.slice(0, 500), - index: alerts.slice(0, 500), + it('400s when attempting to bulk create persistable state attachments reaching the 100 limit', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + await createComment({ + supertest, + caseId: postedCase.id, + params: { + type: CommentType.externalReference as const, + owner: 'securitySolutionFixture', + externalReferenceAttachmentTypeId: '.test', + externalReferenceId: 'so-id', + externalReferenceMetadata: {}, + externalReferenceStorage: { + soType: 'external-ref', + type: ExternalReferenceStorageType.savedObject as const, + }, }, - ], + expectedHttpCode: 200, + }); + + const persistableStateAttachments = Array(100).fill({ + persistableStateAttachmentTypeId: '.test', + persistableStateAttachmentState: {}, + type: CommentType.persistableState as const, + owner: 'securitySolutionFixture', + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: persistableStateAttachments, + expectedHttpCode: 400, + }); }); - await bulkCreateAttachments({ - supertest, - caseId: postedCase.id, - params: [ - { - ...postCommentAlertReq, - alertId: alerts.slice(500), - index: alerts.slice(500), + it('400s when attempting to bulk create >100 external reference attachments reaching the 100 limit', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + await createComment({ + supertest, + caseId: postedCase.id, + params: { + persistableStateAttachmentTypeId: '.test', + persistableStateAttachmentState: {}, + type: CommentType.persistableState as const, + owner: 'securitySolutionFixture', }, - postCommentAlertReq, - ], - expectedHttpCode: 400, + expectedHttpCode: 200, + }); + + const externalRequestAttachments = Array(100).fill({ + type: CommentType.externalReference as const, + owner: 'securitySolutionFixture', + externalReferenceAttachmentTypeId: '.test', + externalReferenceId: 'so-id', + externalReferenceMetadata: {}, + externalReferenceStorage: { + soType: 'external-ref', + type: ExternalReferenceStorageType.savedObject as const, + }, + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: externalRequestAttachments, + expectedHttpCode: 400, + }); }); }); }); From c66c1c8ba0400782f356bc91224c36d697e018f6 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Tue, 25 Jul 2023 12:06:55 +0200 Subject: [PATCH 6/6] Last PR comments. --- .../server/services/attachments/index.test.ts | 84 +++++++++++++++++-- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/cases/server/services/attachments/index.test.ts b/x-pack/plugins/cases/server/services/attachments/index.test.ts index 5967cb6fb2600..610ce7df5bb9d 100644 --- a/x-pack/plugins/cases/server/services/attachments/index.test.ts +++ b/x-pack/plugins/cases/server/services/attachments/index.test.ts @@ -548,12 +548,84 @@ describe('AttachmentService', () => { service.countPersistableStateAndExternalReferenceAttachments({ caseId: 'test-id' }) ).resolves.not.toThrow(); - await expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledWith( - expect.objectContaining({ - hasReference: { id: 'test-id', type: 'cases' }, - type: 'cases-comments', - }) - ); + expect(unsecuredSavedObjectsClient.find.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "filter": Object { + "arguments": Array [ + Object { + "arguments": Array [ + Object { + "arguments": Array [ + Object { + "isQuoted": false, + "type": "literal", + "value": "cases-comments.attributes.type", + }, + Object { + "isQuoted": false, + "type": "literal", + "value": "persistableState", + }, + ], + "function": "is", + "type": "function", + }, + Object { + "arguments": Array [ + Object { + "isQuoted": false, + "type": "literal", + "value": "cases-comments.attributes.type", + }, + Object { + "isQuoted": false, + "type": "literal", + "value": "externalReference", + }, + ], + "function": "is", + "type": "function", + }, + ], + "function": "or", + "type": "function", + }, + Object { + "arguments": Array [ + Object { + "arguments": Array [ + Object { + "isQuoted": false, + "type": "literal", + "value": "cases-comments.attributes.externalReferenceAttachmentTypeId", + }, + Object { + "isQuoted": false, + "type": "literal", + "value": ".files", + }, + ], + "function": "is", + "type": "function", + }, + ], + "function": "not", + "type": "function", + }, + ], + "function": "and", + "type": "function", + }, + "hasReference": Object { + "id": "test-id", + "type": "cases", + }, + "page": 1, + "perPage": 1, + "sortField": "created_at", + "type": "cases-comments", + } + `); }); it('returns the expected total', async () => {