From dd1022236350cd31ac6dc0b397f77911d06aace0 Mon Sep 17 00:00:00 2001 From: Janki Salvi <117571355+js-jankisalvi@users.noreply.github.com> Date: Tue, 4 Jul 2023 12:40:04 +0200 Subject: [PATCH] add guardrails and tests for post and patch comment --- .../cases/common/api/cases/comment/index.ts | 28 ++++- .../plugins/cases/common/constants/index.ts | 1 + .../server/client/attachments/add.test.ts | 29 +++++ .../client/attachments/bulk_create.test.ts | 78 +++++++++++++- .../server/client/attachments/update.test.ts | 100 ++++++++++++++++++ x-pack/plugins/cases/server/mocks.ts | 16 +++ .../tests/common/comments/patch_comment.ts | 67 ++++++++++++ .../tests/common/comments/post_comment.ts | 43 ++++++++ 8 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/cases/server/client/attachments/update.test.ts 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..e733eeadf4e7f 100644 --- a/x-pack/plugins/cases/common/api/cases/comment/index.ts +++ b/x-pack/plugins/cases/common/api/cases/comment/index.ts @@ -10,6 +10,8 @@ import { jsonValueRt } from '../../runtime_types'; import { NumberFromString } from '../../saved_object'; import { UserRt } from '../../user'; +import { limitedStringSchema } from '../../../schema'; +import { MAX_COMMENT_LENGTH } from '../../../constants'; export * from './files'; @@ -192,7 +194,31 @@ const BasicCommentRequestRt = rt.union([ PersistableStateAttachmentRt, ]); -export const CommentRequestRt = rt.union([BasicCommentRequestRt, ExternalReferenceSORt]); +export const CommentRequestRt = rt.union([ + rt.strict({ + comment: limitedStringSchema({ fieldName: 'comment', min: 1, max: MAX_COMMENT_LENGTH }), + type: rt.literal(CommentType.user), + owner: rt.string, + }), + AlertCommentRequestRt, + rt.strict({ + type: rt.literal(CommentType.actions), + comment: limitedStringSchema({ fieldName: 'comment', min: 1, max: MAX_COMMENT_LENGTH }), + actions: rt.strict({ + targets: rt.array( + rt.strict({ + hostname: rt.string, + endpointId: rt.string, + }) + ), + type: rt.string, + }), + owner: rt.string, + }), + ExternalReferenceNoSORt, + ExternalReferenceSORt, + PersistableStateAttachmentRt, +]); export const CommentRequestWithoutRefsRt = rt.union([ BasicCommentRequestRt, diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts index bee8ff9f3b17e..02d4645c8f55d 100644 --- a/x-pack/plugins/cases/common/constants/index.ts +++ b/x-pack/plugins/cases/common/constants/index.ts @@ -117,6 +117,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_DESCRIPTION_LENGTH = 30000 as const; +export const MAX_COMMENT_LENGTH = 30000 as const; export const MAX_LENGTH_PER_TAG = 256 as const; export const MAX_TAGS_PER_CASE = 200 as const; export const MAX_DELETE_IDS_LENGTH = 100 as const; diff --git a/x-pack/plugins/cases/server/client/attachments/add.test.ts b/x-pack/plugins/cases/server/client/attachments/add.test.ts index 0e39ff30a65ec..86646d0c36306 100644 --- a/x-pack/plugins/cases/server/client/attachments/add.test.ts +++ b/x-pack/plugins/cases/server/client/attachments/add.test.ts @@ -7,6 +7,7 @@ import { comment } from '../../mocks'; import { createCasesClientMockArgs } from '../mocks'; +import { MAX_COMMENT_LENGTH } from '../../../common/constants'; import { addComment } from './add'; describe('addComment', () => { @@ -22,4 +23,32 @@ describe('addComment', () => { addComment({ comment: { ...comment, foo: 'bar' }, caseId: 'test-case' }, clientArgs) ).rejects.toThrow('invalid keys "foo"'); }); + + it('should throw an error if the comment length is too long', async () => { + const longComment = Array(MAX_COMMENT_LENGTH + 1) + .fill('x') + .toString(); + + await expect( + addComment({ comment: { ...comment, comment: longComment }, caseId: 'test-case' }, clientArgs) + ).rejects.toThrow( + `Failed while adding a comment to case id: test-case error: Error: The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH}.` + ); + }); + + it('should throw an error if the comment is an empty string', async () => { + await expect( + addComment({ comment: { ...comment, comment: '' }, caseId: 'test-case' }, clientArgs) + ).rejects.toThrow( + 'Failed while adding a comment to case id: test-case error: Error: The comment field cannot be an empty string.' + ); + }); + + it('should throw an error if the description is a string with empty characters', async () => { + await expect( + addComment({ comment: { ...comment, comment: ' ' }, caseId: 'test-case' }, clientArgs) + ).rejects.toThrow( + 'Failed while adding a comment to case id: test-case error: Error: The comment field cannot be an empty string.' + ); + }); }); diff --git a/x-pack/plugins/cases/server/client/attachments/bulk_create.test.ts b/x-pack/plugins/cases/server/client/attachments/bulk_create.test.ts index 7d9cdcf150a20..d9cb3d9ea190b 100644 --- a/x-pack/plugins/cases/server/client/attachments/bulk_create.test.ts +++ b/x-pack/plugins/cases/server/client/attachments/bulk_create.test.ts @@ -5,8 +5,9 @@ * 2.0. */ -import { comment } from '../../mocks'; +import { comment, actionComment } from '../../mocks'; import { createCasesClientMockArgs } from '../mocks'; +import { MAX_COMMENT_LENGTH } from '../../../common/constants'; import { bulkCreate } from './bulk_create'; describe('bulkCreate', () => { @@ -22,4 +23,79 @@ describe('bulkCreate', () => { bulkCreate({ attachments: [{ ...comment, foo: 'bar' }], caseId: 'test-case' }, clientArgs) ).rejects.toThrow('invalid keys "foo"'); }); + + describe('comments', () => { + it('should throw an error if the comment length is too long', async () => { + const longComment = Array(MAX_COMMENT_LENGTH + 1) + .fill('x') + .toString(); + + await expect( + bulkCreate( + { attachments: [{ ...comment, comment: longComment }], caseId: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + `Failed while bulk creating attachment to case id: test-case error: Error: The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH}.` + ); + }); + + it('should throw an error if the comment is an empty string', async () => { + await expect( + bulkCreate({ attachments: [{ ...comment, comment: '' }], caseId: 'test-case' }, clientArgs) + ).rejects.toThrow( + 'Failed while bulk creating attachment to case id: test-case error: Error: The comment field cannot be an empty string.' + ); + }); + + it('should throw an error if the description is a string with empty characters', async () => { + await expect( + bulkCreate( + { attachments: [{ ...comment, comment: ' ' }], caseId: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed while bulk creating attachment to case id: test-case error: Error: The comment field cannot be an empty string.' + ); + }); + }); + + describe('actions', () => { + it('should throw an error if the comment length is too long', async () => { + const longComment = Array(MAX_COMMENT_LENGTH + 1) + .fill('x') + .toString(); + + await expect( + bulkCreate( + { attachments: [{ ...actionComment, comment: longComment }], caseId: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + `Failed while bulk creating attachment to case id: test-case error: Error: The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH}.` + ); + }); + + it('should throw an error if the comment is an empty string', async () => { + await expect( + bulkCreate( + { attachments: [{ ...actionComment, comment: '' }], caseId: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed while bulk creating attachment to case id: test-case error: Error: The comment field cannot be an empty string.' + ); + }); + + it('should throw an error if the description is a string with empty characters', async () => { + await expect( + bulkCreate( + { attachments: [{ ...actionComment, comment: ' ' }], caseId: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed while bulk creating attachment to case id: test-case error: Error: The comment field cannot be an empty string.' + ); + }); + }); }); diff --git a/x-pack/plugins/cases/server/client/attachments/update.test.ts b/x-pack/plugins/cases/server/client/attachments/update.test.ts new file mode 100644 index 0000000000000..c3ebb40d396d5 --- /dev/null +++ b/x-pack/plugins/cases/server/client/attachments/update.test.ts @@ -0,0 +1,100 @@ +/* + * 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 { comment, actionComment } from '../../mocks'; +import { createCasesClientMockArgs } from '../mocks'; +import { MAX_COMMENT_LENGTH } from '../../../common/constants'; +import { update } from './update'; + +describe('update', () => { + const clientArgs = createCasesClientMockArgs(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('comments', () => { + const updateComment = { ...comment, id: 'comment-id', version: 'WzAsMV0=' }; + it('should throw an error if the comment length is too long', async () => { + const longComment = Array(MAX_COMMENT_LENGTH + 1) + .fill('x') + .toString(); + + await expect( + update( + { updateRequest: { ...updateComment, comment: longComment }, caseID: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + `Failed to patch comment case id: test-case: Error: The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH}.` + ); + }); + + it('should throw an error if the comment is an empty string', async () => { + await expect( + update( + { updateRequest: { ...updateComment, comment: '' }, caseID: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed to patch comment case id: test-case: Error: The comment field cannot be an empty string.' + ); + }); + + it('should throw an error if the description is a string with empty characters', async () => { + await expect( + update( + { updateRequest: { ...updateComment, comment: ' ' }, caseID: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed to patch comment case id: test-case: Error: The comment field cannot be an empty string.' + ); + }); + }); + + describe('actions', () => { + const updateActionComment = { ...actionComment, id: 'comment-id', version: 'WzAsMV0=' }; + + it('should throw an error if the comment length is too long', async () => { + const longComment = Array(MAX_COMMENT_LENGTH + 1) + .fill('x') + .toString(); + + await expect( + update( + { updateRequest: { ...updateActionComment, comment: longComment }, caseID: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + `Failed to patch comment case id: test-case: Error: The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH}.` + ); + }); + + it('should throw an error if the comment is an empty string', async () => { + await expect( + update( + { updateRequest: { ...updateActionComment, comment: '' }, caseID: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed to patch comment case id: test-case: Error: The comment field cannot be an empty string.' + ); + }); + + it('should throw an error if the description is a string with empty characters', async () => { + await expect( + update( + { updateRequest: { ...updateActionComment, comment: ' ' }, caseID: 'test-case' }, + clientArgs + ) + ).rejects.toThrow( + 'Failed to patch comment case id: test-case: Error: The comment field cannot be an empty string.' + ); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/mocks.ts b/x-pack/plugins/cases/server/mocks.ts index 479873f7c0a9e..8eacf10c59e15 100644 --- a/x-pack/plugins/cases/server/mocks.ts +++ b/x-pack/plugins/cases/server/mocks.ts @@ -9,6 +9,7 @@ import type { SavedObject } from '@kbn/core/server'; import type { CasePostRequest, CommentAttributes, + CommentRequestActionsType, CommentRequestAlertType, CommentRequestUserType, ConnectorMappings, @@ -664,6 +665,21 @@ export const comment: CommentRequestUserType = { owner: SECURITY_SOLUTION_OWNER, }; +export const actionComment: CommentRequestActionsType = { + type: CommentType.actions, + comment: 'I just isolated the host!', + actions: { + targets: [ + { + hostname: 'host1', + endpointId: '001', + }, + ], + type: 'isolate', + }, + owner: 'cases', +}; + export const alertComment: CommentRequestAlertType = { alertId: 'alert-id-1', index: 'alert-index-1', diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts index fc7d86306fc41..16029e3f84615 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts @@ -173,6 +173,73 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + it('unhappy path - 400s when comment is too long', async () => { + const postedCase = await createCase(supertest, postCaseReq); + const patchedCase = await createComment({ + supertest, + caseId: postedCase.id, + params: postCommentUserReq, + }); + const longComment = Array(30001).fill('a').toString(); + + await updateComment({ + supertest, + caseId: postedCase.id, + req: { + id: patchedCase.comments![0].id, + version: patchedCase.comments![0].version, + type: CommentType.user, + comment: longComment, + owner: 'securitySolutionFixture', + }, + expectedHttpCode: 400, + }); + }); + + it('unhappy path - 400s when comment is empty', async () => { + const postedCase = await createCase(supertest, postCaseReq); + const patchedCase = await createComment({ + supertest, + caseId: postedCase.id, + params: postCommentUserReq, + }); + + await updateComment({ + supertest, + caseId: postedCase.id, + req: { + id: patchedCase.comments![0].id, + version: patchedCase.comments![0].version, + type: CommentType.user, + comment: '', + owner: 'securitySolutionFixture', + }, + expectedHttpCode: 400, + }); + }); + + it('unhappy path - 400s when comment is a string of empty characters', async () => { + const postedCase = await createCase(supertest, postCaseReq); + const patchedCase = await createComment({ + supertest, + caseId: postedCase.id, + params: postCommentUserReq, + }); + + await updateComment({ + supertest, + caseId: postedCase.id, + req: { + id: patchedCase.comments![0].id, + version: patchedCase.comments![0].version, + type: CommentType.user, + comment: ' ', + owner: 'securitySolutionFixture', + }, + expectedHttpCode: 400, + }); + }); + it('unhappy path - 400s when trying to change comment type', async () => { const postedCase = await createCase(supertest, postCaseReq); const patchedCase = await createComment({ 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 34854939b4771..8851a95c6ebc3 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 @@ -274,6 +274,49 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + it('400s when adding too long comment', async () => { + const postedCase = await createCase(supertest, postCaseReq); + const longComment = Array(30001).fill('a').toString(); + + await createComment({ + supertest, + caseId: postedCase.id, + // @ts-expect-error + params: { + comment: longComment, + }, + expectedHttpCode: 400, + }); + }); + + it('400s when adding empty comment', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + await createComment({ + supertest, + caseId: postedCase.id, + // @ts-expect-error + params: { + comment: '', + }, + expectedHttpCode: 400, + }); + }); + + it('400s when adding a comment with only empty characters', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + await createComment({ + supertest, + caseId: postedCase.id, + // @ts-expect-error + params: { + comment: ' ', + }, + expectedHttpCode: 400, + }); + }); + it('400s when adding excess attributes for type user', async () => { const postedCase = await createCase(supertest, postCaseReq);