From 52c6b7b81b2cd7c9d2c50fe4b7a11961db8e3bc1 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 20 Nov 2020 21:03:05 +0200 Subject: [PATCH] [Security Solution][Case] Create comment types (#82715) --- x-pack/plugins/case/README.md | 11 +- .../plugins/case/common/api/cases/comment.ts | 57 +++-- .../case/server/client/comments/add.test.ts | 197 +++++++++++++++-- .../case/server/client/comments/add.ts | 17 +- .../case/server/connectors/case/index.test.ts | 117 +++++++++- .../case/server/connectors/case/schema.ts | 16 +- .../case/server/connectors/case/types.ts | 2 + .../api/__fixtures__/mock_saved_objects.ts | 32 +++ .../api/cases/comments/patch_comment.test.ts | 185 ++++++++++++++++ .../api/cases/comments/patch_comment.ts | 36 ++-- .../api/cases/comments/post_comment.test.ts | 169 +++++++++++++++ .../server/routes/api/cases/get_case.test.ts | 2 +- .../case/server/routes/api/cases/push_case.ts | 9 +- .../case/server/routes/api/utils.test.ts | 6 +- .../plugins/case/server/routes/api/utils.ts | 52 ++++- .../server/saved_object_types/comments.ts | 6 + x-pack/plugins/case/server/services/index.ts | 10 +- .../cases/components/add_comment/index.tsx | 9 +- .../cases/components/add_comment/schema.tsx | 8 +- .../components/user_action_tree/index.tsx | 2 +- .../public/cases/containers/api.test.tsx | 3 +- .../public/cases/containers/api.ts | 12 +- .../public/cases/containers/types.ts | 2 +- .../containers/use_post_comment.test.tsx | 2 +- .../cases/containers/use_post_comment.tsx | 6 +- .../tests/cases/comments/delete_comment.ts | 6 +- .../tests/cases/comments/find_comments.ts | 13 +- .../basic/tests/cases/comments/get_comment.ts | 4 +- .../tests/cases/comments/patch_comment.ts | 170 ++++++++++++++- .../tests/cases/comments/post_comment.ts | 119 +++++++++- .../basic/tests/cases/delete_cases.ts | 4 +- .../basic/tests/cases/find_cases.ts | 6 +- .../basic/tests/cases/push_case.ts | 4 +- .../user_actions/get_all_user_actions.ts | 19 +- .../basic/tests/connectors/case.ts | 204 +++++++++++++++++- .../case_api_integration/common/lib/mock.ts | 13 +- 36 files changed, 1380 insertions(+), 150 deletions(-) diff --git a/x-pack/plugins/case/README.md b/x-pack/plugins/case/README.md index 002fbfb8b53f78..30011148cd1e7b 100644 --- a/x-pack/plugins/case/README.md +++ b/x-pack/plugins/case/README.md @@ -57,11 +57,12 @@ This action type has no `secrets` properties. #### `subActionParams (addComment)` -| Property | Description | Type | -| -------- | --------------------------------------------------------- | ------ | -| comment | The case’s new comment. | string | -| type | The type of the comment, which can be: `user` or `alert`. | string | - +| Property | Description | Type | +| -------- | ----------------------------------------------------------------------- | ----------------- | +| type | The type of the comment | `user` \| `alert` | +| comment | The comment. Valid only when type is `user`. | string | +| alertId | The alert ID. Valid only when the type is `alert` | string | +| index | The index where the alert is saved. Valid only when the type is `alert` | string | #### `connector` | Property | Description | Type | diff --git a/x-pack/plugins/case/common/api/cases/comment.ts b/x-pack/plugins/case/common/api/cases/comment.ts index b4daac93940d88..920858a1e39b4a 100644 --- a/x-pack/plugins/case/common/api/cases/comment.ts +++ b/x-pack/plugins/case/common/api/cases/comment.ts @@ -8,24 +8,33 @@ import * as rt from 'io-ts'; import { UserRT } from '../user'; -const CommentBasicRt = rt.type({ +export const CommentAttributesBasicRt = rt.type({ + created_at: rt.string, + created_by: UserRT, + pushed_at: rt.union([rt.string, rt.null]), + pushed_by: rt.union([UserRT, rt.null]), + updated_at: rt.union([rt.string, rt.null]), + updated_by: rt.union([UserRT, rt.null]), +}); + +export const ContextTypeUserRt = rt.type({ comment: rt.string, - type: rt.union([rt.literal('alert'), rt.literal('user')]), + type: rt.literal('user'), }); -export const CommentAttributesRt = rt.intersection([ - CommentBasicRt, - rt.type({ - created_at: rt.string, - created_by: UserRT, - pushed_at: rt.union([rt.string, rt.null]), - pushed_by: rt.union([UserRT, rt.null]), - updated_at: rt.union([rt.string, rt.null]), - updated_by: rt.union([UserRT, rt.null]), - }), -]); +export const ContextTypeAlertRt = rt.type({ + type: rt.literal('alert'), + alertId: rt.string, + index: rt.string, +}); -export const CommentRequestRt = CommentBasicRt; +const AttributesTypeUserRt = rt.intersection([ContextTypeUserRt, CommentAttributesBasicRt]); +const AttributesTypeAlertsRt = rt.intersection([ContextTypeAlertRt, CommentAttributesBasicRt]); +const CommentAttributesRt = rt.union([AttributesTypeUserRt, AttributesTypeAlertsRt]); + +const ContextBasicRt = rt.union([ContextTypeUserRt, ContextTypeAlertRt]); + +export const CommentRequestRt = ContextBasicRt; export const CommentResponseRt = rt.intersection([ CommentAttributesRt, @@ -38,10 +47,25 @@ export const CommentResponseRt = rt.intersection([ export const AllCommentsResponseRT = rt.array(CommentResponseRt); export const CommentPatchRequestRt = rt.intersection([ - rt.partial(CommentBasicRt.props), + /** + * Partial updates are not allowed. + * We want to prevent the user for changing the type without removing invalid fields. + */ + ContextBasicRt, rt.type({ id: rt.string, version: rt.string }), ]); +/** + * This type is used by the CaseService. + * Because the type for the attributes of savedObjectClient update function is Partial + * we need to make all of our attributes partial too. + * We ensure that partial updates of CommentContext is not going to happen inside the patch comment route. + */ +export const CommentPatchAttributesRt = rt.intersection([ + rt.union([rt.partial(CommentAttributesBasicRt.props), rt.partial(ContextTypeAlertRt.props)]), + rt.partial(CommentAttributesBasicRt.props), +]); + export const CommentsResponseRt = rt.type({ comments: rt.array(CommentResponseRt), page: rt.number, @@ -62,3 +86,6 @@ export type CommentResponse = rt.TypeOf; export type AllCommentsResponse = rt.TypeOf; export type CommentsResponse = rt.TypeOf; export type CommentPatchRequest = rt.TypeOf; +export type CommentPatchAttributes = rt.TypeOf; +export type CommentRequestUserType = rt.TypeOf; +export type CommentRequestAlertType = rt.TypeOf; diff --git a/x-pack/plugins/case/server/client/comments/add.test.ts b/x-pack/plugins/case/server/client/comments/add.test.ts index 50e104b30178ab..d00df5a3246bd2 100644 --- a/x-pack/plugins/case/server/client/comments/add.test.ts +++ b/x-pack/plugins/case/server/client/comments/add.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { omit } from 'lodash/fp'; import { CommentType } from '../../../common/api'; import { createMockSavedObjectsRepository, @@ -31,7 +32,10 @@ describe('addComment', () => { const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); const res = await caseClient.client.addComment({ caseId: 'mock-id-1', - comment: { comment: 'Wow, good luck catching that bad meanie!', type: CommentType.user }, + comment: { + comment: 'Wow, good luck catching that bad meanie!', + type: CommentType.user, + }, }); expect(res.id).toEqual('mock-id-1'); @@ -54,6 +58,43 @@ describe('addComment', () => { }); }); + test('it adds a comment of type alert correctly', async () => { + const savedObjectsClient = createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }); + + const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); + const res = await caseClient.client.addComment({ + caseId: 'mock-id-1', + comment: { + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + }, + }); + + expect(res.id).toEqual('mock-id-1'); + expect(res.totalComment).toEqual(res.comments!.length); + expect(res.comments![res.comments!.length - 1]).toEqual({ + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + created_at: '2020-10-23T21:54:48.952Z', + created_by: { + email: 'd00d@awesome.com', + full_name: 'Awesome D00d', + username: 'awesome', + }, + id: 'mock-comment', + pushed_at: null, + pushed_by: null, + updated_at: null, + updated_by: null, + version: 'WzksMV0=', + }); + }); + test('it updates the case correctly after adding a comment', async () => { const savedObjectsClient = createMockSavedObjectsRepository({ caseSavedObject: mockCases, @@ -63,7 +104,10 @@ describe('addComment', () => { const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); const res = await caseClient.client.addComment({ caseId: 'mock-id-1', - comment: { comment: 'Wow, good luck catching that bad meanie!', type: CommentType.user }, + comment: { + comment: 'Wow, good luck catching that bad meanie!', + type: CommentType.user, + }, }); expect(res.updated_at).toEqual('2020-10-23T21:54:48.952Z'); @@ -83,7 +127,10 @@ describe('addComment', () => { const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); await caseClient.client.addComment({ caseId: 'mock-id-1', - comment: { comment: 'Wow, good luck catching that bad meanie!', type: CommentType.user }, + comment: { + comment: 'Wow, good luck catching that bad meanie!', + type: CommentType.user, + }, }); expect( @@ -99,7 +146,7 @@ describe('addComment', () => { username: 'awesome', }, action_field: ['comment'], - new_value: 'Wow, good luck catching that bad meanie!', + new_value: '{"comment":"Wow, good luck catching that bad meanie!","type":"user"}', old_value: null, }, references: [ @@ -127,7 +174,10 @@ describe('addComment', () => { const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient, true); const res = await caseClient.client.addComment({ caseId: 'mock-id-1', - comment: { comment: 'Wow, good luck catching that bad meanie!', type: CommentType.user }, + comment: { + comment: 'Wow, good luck catching that bad meanie!', + type: CommentType.user, + }, }); expect(res.id).toEqual('mock-id-1'); @@ -151,7 +201,7 @@ describe('addComment', () => { }); describe('unhappy path', () => { - test('it throws when missing comment', async () => { + test('it throws when missing type', async () => { expect.assertions(3); const savedObjectsClient = createMockSavedObjectsRepository({ @@ -172,25 +222,126 @@ describe('addComment', () => { }); }); - test('it throws when missing comment type', async () => { + test('it throws when missing attributes: type user', async () => { expect.assertions(3); const savedObjectsClient = createMockSavedObjectsRepository({ caseSavedObject: mockCases, caseCommentSavedObject: mockCaseComments, }); + const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); - caseClient.client - .addComment({ - caseId: 'mock-id-1', - // @ts-expect-error - comment: { comment: 'a comment' }, - }) - .catch((e) => { - expect(e).not.toBeNull(); - expect(e.isBoom).toBe(true); - expect(e.output.statusCode).toBe(400); - }); + const allRequestAttributes = { + type: CommentType.user, + comment: 'a comment', + }; + + ['comment'].forEach((attribute) => { + const requestAttributes = omit(attribute, allRequestAttributes); + caseClient.client + .addComment({ + caseId: 'mock-id-1', + // @ts-expect-error + comment: { + ...requestAttributes, + }, + }) + .catch((e) => { + expect(e).not.toBeNull(); + expect(e.isBoom).toBe(true); + expect(e.output.statusCode).toBe(400); + }); + }); + }); + + test('it throws when excess attributes are provided: type user', async () => { + expect.assertions(6); + + const savedObjectsClient = createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }); + + const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); + + ['alertId', 'index'].forEach((attribute) => { + caseClient.client + .addComment({ + caseId: 'mock-id-1', + comment: { + [attribute]: attribute, + comment: 'a comment', + type: CommentType.user, + }, + }) + .catch((e) => { + expect(e).not.toBeNull(); + expect(e.isBoom).toBe(true); + expect(e.output.statusCode).toBe(400); + }); + }); + }); + + test('it throws when missing attributes: type alert', async () => { + expect.assertions(6); + + const savedObjectsClient = createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }); + + const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); + const allRequestAttributes = { + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }; + + ['alertId', 'index'].forEach((attribute) => { + const requestAttributes = omit(attribute, allRequestAttributes); + caseClient.client + .addComment({ + caseId: 'mock-id-1', + // @ts-expect-error + comment: { + ...requestAttributes, + }, + }) + .catch((e) => { + expect(e).not.toBeNull(); + expect(e.isBoom).toBe(true); + expect(e.output.statusCode).toBe(400); + }); + }); + }); + + test('it throws when excess attributes are provided: type alert', async () => { + expect.assertions(3); + + const savedObjectsClient = createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }); + + const caseClient = await createCaseClientWithMockSavedObjectsClient(savedObjectsClient); + + ['comment'].forEach((attribute) => { + caseClient.client + .addComment({ + caseId: 'mock-id-1', + comment: { + [attribute]: attribute, + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }, + }) + .catch((e) => { + expect(e).not.toBeNull(); + expect(e.isBoom).toBe(true); + expect(e.output.statusCode).toBe(400); + }); + }); }); test('it throws when the case does not exists', async () => { @@ -204,7 +355,10 @@ describe('addComment', () => { caseClient.client .addComment({ caseId: 'not-exists', - comment: { comment: 'Wow, good luck catching that bad meanie!', type: CommentType.user }, + comment: { + comment: 'Wow, good luck catching that bad meanie!', + type: CommentType.user, + }, }) .catch((e) => { expect(e).not.toBeNull(); @@ -224,7 +378,10 @@ describe('addComment', () => { caseClient.client .addComment({ caseId: 'mock-id-1', - comment: { comment: 'Throw an error', type: CommentType.user }, + comment: { + comment: 'Throw an error', + type: CommentType.user, + }, }) .catch((e) => { expect(e).not.toBeNull(); diff --git a/x-pack/plugins/case/server/client/comments/add.ts b/x-pack/plugins/case/server/client/comments/add.ts index a95b7833a5232f..169157c95d4c1a 100644 --- a/x-pack/plugins/case/server/client/comments/add.ts +++ b/x-pack/plugins/case/server/client/comments/add.ts @@ -9,15 +9,9 @@ import { pipe } from 'fp-ts/lib/pipeable'; import { fold } from 'fp-ts/lib/Either'; import { identity } from 'fp-ts/lib/function'; -import { flattenCaseSavedObject, transformNewComment } from '../../routes/api/utils'; +import { decodeComment, flattenCaseSavedObject, transformNewComment } from '../../routes/api/utils'; -import { - throwErrors, - excess, - CaseResponseRt, - CommentRequestRt, - CaseResponse, -} from '../../../common/api'; +import { throwErrors, CaseResponseRt, CommentRequestRt, CaseResponse } from '../../../common/api'; import { buildCommentUserActionItem } from '../../services/user_actions/helpers'; import { CaseClientAddComment, CaseClientFactoryArguments } from '../types'; @@ -33,10 +27,13 @@ export const addComment = ({ comment, }: CaseClientAddComment): Promise => { const query = pipe( - excess(CommentRequestRt).decode(comment), + // TODO: Excess CommentRequestRt when the excess() function supports union types + CommentRequestRt.decode(comment), fold(throwErrors(Boom.badRequest), identity) ); + decodeComment(comment); + const myCase = await caseService.getCase({ client: savedObjectsClient, caseId, @@ -105,7 +102,7 @@ export const addComment = ({ caseId: myCase.id, commentId: newComment.id, fields: ['comment'], - newValue: query.comment, + newValue: JSON.stringify(query), }), ], }), diff --git a/x-pack/plugins/case/server/connectors/case/index.test.ts b/x-pack/plugins/case/server/connectors/case/index.test.ts index e14281e047915a..90bb1d604e7330 100644 --- a/x-pack/plugins/case/server/connectors/case/index.test.ts +++ b/x-pack/plugins/case/server/connectors/case/index.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { omit } from 'lodash/fp'; import { Logger } from '../../../../../../src/core/server'; import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { actionsMock } from '../../../../actions/server/mocks'; @@ -614,12 +615,31 @@ describe('case connector', () => { }); describe('add comment', () => { - it('succeeds when params is valid', () => { + it('succeeds when type is user', () => { + const params: Record = { + subAction: 'addComment', + subActionParams: { + caseId: 'case-id', + comment: { + comment: 'a comment', + type: CommentType.user, + }, + }, + }; + + expect(validateParams(caseActionType, params)).toEqual(params); + }); + + it('succeeds when type is an alert', () => { const params: Record = { subAction: 'addComment', subActionParams: { caseId: 'case-id', - comment: { comment: 'a comment', type: CommentType.user }, + comment: { + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + }, }, }; @@ -635,6 +655,89 @@ describe('case connector', () => { validateParams(caseActionType, params); }).toThrow(); }); + + it('fails when missing attributes: type user', () => { + const allParams = { + type: CommentType.user, + comment: 'a comment', + }; + + ['comment'].forEach((attribute) => { + const comment = omit(attribute, allParams); + const params: Record = { + subAction: 'addComment', + subActionParams: { + caseId: 'case-id', + comment, + }, + }; + + expect(() => { + validateParams(caseActionType, params); + }).toThrow(); + }); + }); + + it('fails when missing attributes: type alert', () => { + const allParams = { + type: CommentType.alert, + comment: 'a comment', + alertId: 'test-id', + index: 'test-index', + }; + + ['alertId', 'index'].forEach((attribute) => { + const comment = omit(attribute, allParams); + const params: Record = { + subAction: 'addComment', + subActionParams: { + caseId: 'case-id', + comment, + }, + }; + + expect(() => { + validateParams(caseActionType, params); + }).toThrow(); + }); + }); + + it('fails when excess attributes are provided: type user', () => { + ['alertId', 'index'].forEach((attribute) => { + const params: Record = { + subAction: 'addComment', + subActionParams: { + caseId: 'case-id', + [attribute]: attribute, + type: CommentType.user, + comment: 'a comment', + }, + }; + + expect(() => { + validateParams(caseActionType, params); + }).toThrow(); + }); + }); + + it('fails when excess attributes are provided: type alert', () => { + ['comment'].forEach((attribute) => { + const params: Record = { + subAction: 'addComment', + subActionParams: { + caseId: 'case-id', + [attribute]: attribute, + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + }, + }; + + expect(() => { + validateParams(caseActionType, params); + }).toThrow(); + }); + }); }); }); @@ -866,7 +969,10 @@ describe('case connector', () => { subAction: 'addComment', subActionParams: { caseId: 'case-id', - comment: { comment: 'a comment', type: CommentType.user }, + comment: { + comment: 'a comment', + type: CommentType.user, + }, }, }; @@ -883,7 +989,10 @@ describe('case connector', () => { expect(result).toEqual({ actionId, status: 'ok', data: commentReturn }); expect(mockCaseClient.addComment).toHaveBeenCalledWith({ caseId: 'case-id', - comment: { comment: 'a comment', type: CommentType.user }, + comment: { + comment: 'a comment', + type: CommentType.user, + }, }); }); }); diff --git a/x-pack/plugins/case/server/connectors/case/schema.ts b/x-pack/plugins/case/server/connectors/case/schema.ts index aa503e96be30d3..039c0e2e7e67f1 100644 --- a/x-pack/plugins/case/server/connectors/case/schema.ts +++ b/x-pack/plugins/case/server/connectors/case/schema.ts @@ -9,10 +9,18 @@ import { validateConnector } from './validators'; // Reserved for future implementation export const CaseConfigurationSchema = schema.object({}); -const CommentProps = { +const ContextTypeUserSchema = schema.object({ + type: schema.literal('user'), comment: schema.string(), - type: schema.oneOf([schema.literal('alert'), schema.literal('user')]), -}; +}); + +const ContextTypeAlertSchema = schema.object({ + type: schema.literal('alert'), + alertId: schema.string(), + index: schema.string(), +}); + +export const CommentSchema = schema.oneOf([ContextTypeUserSchema, ContextTypeAlertSchema]); const JiraFieldsSchema = schema.object({ issueType: schema.string(), @@ -86,7 +94,7 @@ const CaseUpdateRequestProps = { const CaseAddCommentRequestProps = { caseId: schema.string(), - comment: schema.object(CommentProps), + comment: CommentSchema, }; export const ExecutorSubActionCreateParamsSchema = schema.object(CaseBasicProps); diff --git a/x-pack/plugins/case/server/connectors/case/types.ts b/x-pack/plugins/case/server/connectors/case/types.ts index b3a05163fa6f4d..da15f64a5718f5 100644 --- a/x-pack/plugins/case/server/connectors/case/types.ts +++ b/x-pack/plugins/case/server/connectors/case/types.ts @@ -13,11 +13,13 @@ import { CaseConfigurationSchema, ExecutorSubActionAddCommentParamsSchema, ConnectorSchema, + CommentSchema, } from './schema'; import { CaseResponse, CasesResponse } from '../../../common/api'; export type CaseConfiguration = TypeOf; export type Connector = TypeOf; +export type Comment = TypeOf; export type ExecutorSubActionCreateParams = TypeOf; export type ExecutorSubActionUpdateParams = TypeOf; diff --git a/x-pack/plugins/case/server/routes/api/__fixtures__/mock_saved_objects.ts b/x-pack/plugins/case/server/routes/api/__fixtures__/mock_saved_objects.ts index 9314ebb445820f..4c0b5887ca9988 100644 --- a/x-pack/plugins/case/server/routes/api/__fixtures__/mock_saved_objects.ts +++ b/x-pack/plugins/case/server/routes/api/__fixtures__/mock_saved_objects.ts @@ -297,6 +297,38 @@ export const mockCaseComments: Array> = [ updated_at: '2019-11-25T22:32:30.608Z', version: 'WzYsMV0=', }, + { + type: 'cases-comment', + id: 'mock-comment-4', + attributes: { + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + created_at: '2019-11-25T22:32:30.608Z', + created_by: { + full_name: 'elastic', + email: 'testemail@elastic.co', + username: 'elastic', + }, + pushed_at: null, + pushed_by: null, + updated_at: '2019-11-25T22:32:30.608Z', + updated_by: { + full_name: 'elastic', + email: 'testemail@elastic.co', + username: 'elastic', + }, + }, + references: [ + { + type: 'cases', + name: 'associated-cases', + id: 'mock-id-4', + }, + ], + updated_at: '2019-11-25T22:32:30.608Z', + version: 'WzYsMV0=', + }, ]; export const mockCaseConfigure: Array> = [ diff --git a/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.test.ts b/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.test.ts index 400e8ca404ca5e..5cb411f17a7448 100644 --- a/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.test.ts +++ b/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.test.ts @@ -3,6 +3,8 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ + +import { omit } from 'lodash/fp'; import { kibanaResponseFactory, RequestHandler } from 'src/core/server'; import { httpServerMock } from 'src/core/server/mocks'; @@ -15,12 +17,14 @@ import { } from '../../__fixtures__'; import { initPatchCommentApi } from './patch_comment'; import { CASE_COMMENTS_URL } from '../../../../../common/constants'; +import { CommentType } from '../../../../../common/api'; describe('PATCH comment', () => { let routeHandler: RequestHandler; beforeAll(async () => { routeHandler = await createRoute(initPatchCommentApi, 'patch'); }); + it(`Patch a comment`, async () => { const request = httpServerMock.createKibanaRequest({ path: CASE_COMMENTS_URL, @@ -29,6 +33,7 @@ describe('PATCH comment', () => { case_id: 'mock-id-1', }, body: { + type: CommentType.user, comment: 'Update my comment', id: 'mock-comment-1', version: 'WzEsMV0=', @@ -49,6 +54,183 @@ describe('PATCH comment', () => { ); }); + it(`Patch an alert`, async () => { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'patch', + params: { + case_id: 'mock-id-4', + }, + body: { + type: CommentType.alert, + alertId: 'new-id', + index: 'test-index', + id: 'mock-comment-4', + version: 'WzYsMV0=', + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(200); + expect(response.payload.comments[response.payload.comments.length - 1].alertId).toEqual( + 'new-id' + ); + }); + + it(`it throws when missing attributes: type user`, async () => { + const allRequestAttributes = { + type: CommentType.user, + comment: 'a comment', + }; + + for (const attribute of ['comment']) { + const requestAttributes = omit(attribute, allRequestAttributes); + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: requestAttributes, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it throws when excess attributes are provided: type user`, async () => { + for (const attribute of ['alertId', 'index']) { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: { + [attribute]: attribute, + comment: 'a comment', + type: CommentType.user, + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it throws when missing attributes: type alert`, async () => { + const allRequestAttributes = { + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }; + + for (const attribute of ['alertId', 'index']) { + const requestAttributes = omit(attribute, allRequestAttributes); + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: requestAttributes, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it throws when excess attributes are provided: type alert`, async () => { + for (const attribute of ['comment']) { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: { + [attribute]: attribute, + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it fails to change the type of the comment`, async () => { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'patch', + params: { + case_id: 'mock-id-1', + }, + body: { + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + id: 'mock-comment-1', + version: 'WzEsMV0=', + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + expect(response.payload.message).toEqual('You cannot change the type of the comment.'); + }); + it(`Fails with 409 if version does not match`, async () => { const request = httpServerMock.createKibanaRequest({ path: CASE_COMMENTS_URL, @@ -57,6 +239,7 @@ describe('PATCH comment', () => { case_id: 'mock-id-1', }, body: { + type: CommentType.user, id: 'mock-comment-1', comment: 'Update my comment', version: 'badv=', @@ -73,6 +256,7 @@ describe('PATCH comment', () => { const response = await routeHandler(theContext, request, kibanaResponseFactory); expect(response.status).toEqual(409); }); + it(`Returns an error if updateComment throws`, async () => { const request = httpServerMock.createKibanaRequest({ path: CASE_COMMENTS_URL, @@ -81,6 +265,7 @@ describe('PATCH comment', () => { case_id: 'mock-id-1', }, body: { + type: CommentType.user, comment: 'Update my comment', id: 'mock-comment-does-not-exist', version: 'WzEsMV0=', diff --git a/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.ts b/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.ts index e75e89fa207b91..82fe3fce67653c 100644 --- a/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.ts +++ b/x-pack/plugins/case/server/routes/api/cases/comments/patch_comment.ts @@ -4,17 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ -import { schema } from '@kbn/config-schema'; -import Boom from '@hapi/boom'; +import { pick } from 'lodash/fp'; import { pipe } from 'fp-ts/lib/pipeable'; import { fold } from 'fp-ts/lib/Either'; import { identity } from 'fp-ts/lib/function'; +import { schema } from '@kbn/config-schema'; +import Boom from '@hapi/boom'; import { CommentPatchRequestRt, CaseResponseRt, throwErrors } from '../../../../../common/api'; import { CASE_SAVED_OBJECT } from '../../../../saved_object_types'; import { buildCommentUserActionItem } from '../../../../services/user_actions/helpers'; import { RouteDeps } from '../../types'; -import { escapeHatch, wrapError, flattenCaseSavedObject } from '../../utils'; +import { escapeHatch, wrapError, flattenCaseSavedObject, decodeComment } from '../../utils'; import { CASE_COMMENTS_URL } from '../../../../../common/constants'; export function initPatchCommentApi({ @@ -42,6 +43,9 @@ export function initPatchCommentApi({ fold(throwErrors(Boom.badRequest), identity) ); + const { id: queryCommentId, version: queryCommentVersion, ...queryRestAttributes } = query; + decodeComment(queryRestAttributes); + const myCase = await caseService.getCase({ client, caseId, @@ -49,19 +53,23 @@ export function initPatchCommentApi({ const myComment = await caseService.getComment({ client, - commentId: query.id, + commentId: queryCommentId, }); if (myComment == null) { - throw Boom.notFound(`This comment ${query.id} does not exist anymore.`); + throw Boom.notFound(`This comment ${queryCommentId} does not exist anymore.`); + } + + if (myComment.attributes.type !== queryRestAttributes.type) { + throw Boom.badRequest(`You cannot change the type of the comment.`); } const caseRef = myComment.references.find((c) => c.type === CASE_SAVED_OBJECT); if (caseRef == null || (caseRef != null && caseRef.id !== caseId)) { - throw Boom.notFound(`This comment ${query.id} does not exist in ${caseId}).`); + throw Boom.notFound(`This comment ${queryCommentId} does not exist in ${caseId}).`); } - if (query.version !== myComment.version) { + if (queryCommentVersion !== myComment.version) { throw Boom.conflict( 'This case has been updated. Please refresh before saving additional updates.' ); @@ -73,13 +81,13 @@ export function initPatchCommentApi({ const [updatedComment, updatedCase] = await Promise.all([ caseService.patchComment({ client, - commentId: query.id, + commentId: queryCommentId, updatedAttributes: { - comment: query.comment, + ...queryRestAttributes, updated_at: updatedDate, updated_by: { email, full_name, username }, }, - version: query.version, + version: queryCommentVersion, }), caseService.patchCase({ client, @@ -122,8 +130,12 @@ export function initPatchCommentApi({ caseId: request.params.case_id, commentId: updatedComment.id, fields: ['comment'], - newValue: query.comment, - oldValue: myComment.attributes.comment, + newValue: JSON.stringify(queryRestAttributes), + oldValue: JSON.stringify( + // We are interested only in ContextBasicRt attributes + // myComment.attribute contains also CommentAttributesBasicRt attributes + pick(Object.keys(queryRestAttributes), myComment.attributes) + ), }), ], }), diff --git a/x-pack/plugins/case/server/routes/api/cases/comments/post_comment.test.ts b/x-pack/plugins/case/server/routes/api/cases/comments/post_comment.test.ts index 0b733bb034f8cc..2909aa40a44252 100644 --- a/x-pack/plugins/case/server/routes/api/cases/comments/post_comment.test.ts +++ b/x-pack/plugins/case/server/routes/api/cases/comments/post_comment.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { omit } from 'lodash/fp'; import { kibanaResponseFactory, RequestHandler } from 'src/core/server'; import { httpServerMock } from 'src/core/server/mocks'; @@ -55,6 +56,174 @@ describe('POST comment', () => { ); }); + it(`Posts a new comment of type alert`, async () => { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: { + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(200); + expect(response.payload.comments[response.payload.comments.length - 1].id).toEqual( + 'mock-comment' + ); + }); + + it(`it throws when missing type`, async () => { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: {}, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + }); + + it(`it throws when missing attributes: type user`, async () => { + const allRequestAttributes = { + type: CommentType.user, + comment: 'a comment', + }; + + for (const attribute of ['comment']) { + const requestAttributes = omit(attribute, allRequestAttributes); + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: requestAttributes, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it throws when excess attributes are provided: type user`, async () => { + for (const attribute of ['alertId', 'index']) { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: { + [attribute]: attribute, + comment: 'a comment', + type: CommentType.user, + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it throws when missing attributes: type alert`, async () => { + const allRequestAttributes = { + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }; + + for (const attribute of ['alertId', 'index']) { + const requestAttributes = omit(attribute, allRequestAttributes); + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: requestAttributes, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + + it(`it throws when excess attributes are provided: type alert`, async () => { + for (const attribute of ['comment']) { + const request = httpServerMock.createKibanaRequest({ + path: CASE_COMMENTS_URL, + method: 'post', + params: { + case_id: 'mock-id-1', + }, + body: { + [attribute]: attribute, + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }, + }); + + const theContext = await createRouteContext( + createMockSavedObjectsRepository({ + caseSavedObject: mockCases, + caseCommentSavedObject: mockCaseComments, + }) + ); + + const response = await routeHandler(theContext, request, kibanaResponseFactory); + expect(response.status).toEqual(400); + expect(response.payload.isBoom).toEqual(true); + } + }); + it(`Returns an error if the case does not exist`, async () => { const request = httpServerMock.createKibanaRequest({ path: CASE_COMMENTS_URL, diff --git a/x-pack/plugins/case/server/routes/api/cases/get_case.test.ts b/x-pack/plugins/case/server/routes/api/cases/get_case.test.ts index 01de9abac16afe..6e2dfdc59f1b1b 100644 --- a/x-pack/plugins/case/server/routes/api/cases/get_case.test.ts +++ b/x-pack/plugins/case/server/routes/api/cases/get_case.test.ts @@ -104,7 +104,7 @@ describe('GET case', () => { const response = await routeHandler(theContext, request, kibanaResponseFactory); expect(response.status).toEqual(200); - expect(response.payload.comments).toHaveLength(3); + expect(response.payload.comments).toHaveLength(4); }); it(`returns an error when thrown from getAllCaseComments`, async () => { diff --git a/x-pack/plugins/case/server/routes/api/cases/push_case.ts b/x-pack/plugins/case/server/routes/api/cases/push_case.ts index 80b65b54468fcb..6ba2da111090f7 100644 --- a/x-pack/plugins/case/server/routes/api/cases/push_case.ts +++ b/x-pack/plugins/case/server/routes/api/cases/push_case.ts @@ -11,7 +11,12 @@ import { pipe } from 'fp-ts/lib/pipeable'; import { fold } from 'fp-ts/lib/Either'; import { identity } from 'fp-ts/lib/function'; -import { flattenCaseSavedObject, wrapError, escapeHatch } from '../utils'; +import { + flattenCaseSavedObject, + wrapError, + escapeHatch, + getCommentContextFromAttributes, +} from '../utils'; import { CaseExternalServiceRequestRt, CaseResponseRt, throwErrors } from '../../../../common/api'; import { buildCaseUserActionItem } from '../../../services/user_actions/helpers'; @@ -164,6 +169,7 @@ export function initPushCaseUserActionApi({ ], }), ]); + return response.ok({ body: CaseResponseRt.encode( flattenCaseSavedObject({ @@ -183,6 +189,7 @@ export function initPushCaseUserActionApi({ attributes: { ...origComment.attributes, ...updatedComment?.attributes, + ...getCommentContextFromAttributes(origComment.attributes), }, version: updatedComment?.version ?? origComment.version, references: origComment?.references ?? [], diff --git a/x-pack/plugins/case/server/routes/api/utils.test.ts b/x-pack/plugins/case/server/routes/api/utils.test.ts index fc1086b03814b6..a67bae5ed74dc9 100644 --- a/x-pack/plugins/case/server/routes/api/utils.test.ts +++ b/x-pack/plugins/case/server/routes/api/utils.test.ts @@ -117,7 +117,7 @@ describe('Utils', () => { it('transforms correctly', () => { const comment = { comment: 'A comment', - type: CommentType.user, + type: CommentType.user as const, createdDate: '2020-04-09T09:43:51.778Z', email: 'elastic@elastic.co', full_name: 'Elastic', @@ -140,7 +140,7 @@ describe('Utils', () => { it('transform correctly without optional fields', () => { const comment = { comment: 'A comment', - type: CommentType.user, + type: CommentType.user as const, createdDate: '2020-04-09T09:43:51.778Z', }; @@ -161,7 +161,7 @@ describe('Utils', () => { it('transform correctly with optional fields as null', () => { const comment = { comment: 'A comment', - type: CommentType.user, + type: CommentType.user as const, createdDate: '2020-04-09T09:43:51.778Z', email: null, full_name: null, diff --git a/x-pack/plugins/case/server/routes/api/utils.ts b/x-pack/plugins/case/server/routes/api/utils.ts index f8fe149c2ff2f6..589d7c02a7be60 100644 --- a/x-pack/plugins/case/server/routes/api/utils.ts +++ b/x-pack/plugins/case/server/routes/api/utils.ts @@ -4,8 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ +import { badRequest, boomify, isBoom } from '@hapi/boom'; +import { fold } from 'fp-ts/lib/Either'; +import { identity } from 'fp-ts/lib/function'; +import { pipe } from 'fp-ts/lib/pipeable'; import { schema } from '@kbn/config-schema'; -import { boomify, isBoom } from '@hapi/boom'; import { CustomHttpResponseOptions, ResponseError, @@ -23,6 +26,13 @@ import { ESCaseConnector, ESCaseAttributes, CommentRequest, + ContextTypeUserRt, + ContextTypeAlertRt, + CommentRequestUserType, + CommentRequestAlertType, + CommentType, + excess, + throwErrors, } from '../../../common/api'; import { transformESConnectorToCaseConnector } from './cases/helpers'; @@ -56,24 +66,22 @@ export const transformNewCase = ({ updated_by: null, }); -interface NewCommentArgs extends CommentRequest { +type NewCommentArgs = CommentRequest & { createdDate: string; email?: string | null; full_name?: string | null; username?: string | null; -} +}; export const transformNewComment = ({ - comment, - type, createdDate, email, // eslint-disable-next-line @typescript-eslint/naming-convention full_name, username, + ...comment }: NewCommentArgs): CommentAttributes => ({ - comment, - type, + ...comment, created_at: createdDate, created_by: { email, full_name, username }, pushed_at: null, @@ -178,3 +186,33 @@ export const sortToSnake = (sortField: string): SortFieldCase => { }; export const escapeHatch = schema.object({}, { unknowns: 'allow' }); + +const isUserContext = (context: CommentRequest): context is CommentRequestUserType => { + return context.type === CommentType.user; +}; + +const isAlertContext = (context: CommentRequest): context is CommentRequestAlertType => { + return context.type === CommentType.alert; +}; + +export const decodeComment = (comment: CommentRequest) => { + if (isUserContext(comment)) { + pipe(excess(ContextTypeUserRt).decode(comment), fold(throwErrors(badRequest), identity)); + } else if (isAlertContext(comment)) { + pipe(excess(ContextTypeAlertRt).decode(comment), fold(throwErrors(badRequest), identity)); + } +}; + +export const getCommentContextFromAttributes = ( + attributes: CommentAttributes +): CommentRequestUserType | CommentRequestAlertType => + isUserContext(attributes) + ? { + type: CommentType.user, + comment: attributes.comment, + } + : { + type: CommentType.alert, + alertId: attributes.alertId, + index: attributes.index, + }; diff --git a/x-pack/plugins/case/server/saved_object_types/comments.ts b/x-pack/plugins/case/server/saved_object_types/comments.ts index 87478eb23641f3..8f398c63e01bd5 100644 --- a/x-pack/plugins/case/server/saved_object_types/comments.ts +++ b/x-pack/plugins/case/server/saved_object_types/comments.ts @@ -21,6 +21,12 @@ export const caseCommentSavedObjectType: SavedObjectsType = { type: { type: 'keyword', }, + alertId: { + type: 'keyword', + }, + index: { + type: 'keyword', + }, created_at: { type: 'date', }, diff --git a/x-pack/plugins/case/server/services/index.ts b/x-pack/plugins/case/server/services/index.ts index cab8cb499c3fae..0ce2b196af471e 100644 --- a/x-pack/plugins/case/server/services/index.ts +++ b/x-pack/plugins/case/server/services/index.ts @@ -23,6 +23,7 @@ import { CommentAttributes, SavedObjectFindOptions, User, + CommentPatchAttributes, } from '../../common/api'; import { CASE_SAVED_OBJECT, CASE_COMMENT_SAVED_OBJECT } from '../saved_object_types'; import { readReporters } from './reporters/read_reporters'; @@ -78,18 +79,15 @@ type PatchCaseArgs = PatchCase & ClientArgs; interface PatchCasesArgs extends ClientArgs { cases: PatchCase[]; } -interface UpdateCommentArgs extends ClientArgs { - commentId: string; - updatedAttributes: Partial; - version?: string; -} interface PatchComment { commentId: string; - updatedAttributes: Partial; + updatedAttributes: CommentPatchAttributes; version?: string; } +type UpdateCommentArgs = PatchComment & ClientArgs; + interface PatchComments extends ClientArgs { comments: PatchComment[]; } diff --git a/x-pack/plugins/security_solution/public/cases/components/add_comment/index.tsx b/x-pack/plugins/security_solution/public/cases/components/add_comment/index.tsx index c54bd8b621d835..859ba3d1a0951d 100644 --- a/x-pack/plugins/security_solution/public/cases/components/add_comment/index.tsx +++ b/x-pack/plugins/security_solution/public/cases/components/add_comment/index.tsx @@ -8,7 +8,7 @@ import { EuiButton, EuiLoadingSpinner } from '@elastic/eui'; import React, { useCallback, forwardRef, useImperativeHandle } from 'react'; import styled from 'styled-components'; -import { CommentRequest, CommentType } from '../../../../../case/common/api'; +import { CommentType } from '../../../../../case/common/api'; import { usePostComment } from '../../containers/use_post_comment'; import { Case } from '../../containers/types'; import { MarkdownEditorForm } from '../../../common/components/markdown_editor/eui_form'; @@ -16,7 +16,7 @@ import { useInsertTimeline } from '../../../timelines/components/timeline/insert import { Form, useForm, UseField, useFormData } from '../../../shared_imports'; import * as i18n from './translations'; -import { schema } from './schema'; +import { schema, AddCommentFormSchema } from './schema'; import { useTimelineClick } from '../../../common/utils/timeline/use_timeline_click'; const MySpinner = styled(EuiLoadingSpinner)` @@ -25,9 +25,8 @@ const MySpinner = styled(EuiLoadingSpinner)` left: 50%; `; -const initialCommentValue: CommentRequest = { +const initialCommentValue: AddCommentFormSchema = { comment: '', - type: CommentType.user, }; export interface AddCommentRefObject { @@ -47,7 +46,7 @@ export const AddComment = React.memo( ({ caseId, disabled, showLoading = true, onCommentPosted, onCommentSaving }, ref) => { const { isLoading, postComment } = usePostComment(caseId); - const { form } = useForm({ + const { form } = useForm({ defaultValue: initialCommentValue, options: { stripEmptyFields: false }, schema, diff --git a/x-pack/plugins/security_solution/public/cases/components/add_comment/schema.tsx b/x-pack/plugins/security_solution/public/cases/components/add_comment/schema.tsx index eb11357cd7ce94..5f244d64701fe4 100644 --- a/x-pack/plugins/security_solution/public/cases/components/add_comment/schema.tsx +++ b/x-pack/plugins/security_solution/public/cases/components/add_comment/schema.tsx @@ -4,13 +4,17 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CommentRequest } from '../../../../../case/common/api'; +import { CommentRequestUserType } from '../../../../../case/common/api'; import { FIELD_TYPES, fieldValidators, FormSchema } from '../../../shared_imports'; import * as i18n from './translations'; const { emptyField } = fieldValidators; -export const schema: FormSchema = { +export interface AddCommentFormSchema { + comment: CommentRequestUserType['comment']; +} + +export const schema: FormSchema = { comment: { type: FIELD_TYPES.TEXTAREA, validations: [ diff --git a/x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.tsx b/x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.tsx index de3e9c07ae8a38..228f3a4319c338 100644 --- a/x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.tsx +++ b/x-pack/plugins/security_solution/public/cases/components/user_action_tree/index.tsx @@ -380,7 +380,7 @@ export const UserActionTree = React.memo( ]; } - // description, comments, tags + // title, description, comments, tags if ( action.actionField.length === 1 && ['title', 'description', 'comment', 'tags'].includes(action.actionField[0]) diff --git a/x-pack/plugins/security_solution/public/cases/containers/api.test.tsx b/x-pack/plugins/security_solution/public/cases/containers/api.test.tsx index 0d5bf13cd62618..0d2df7c2de3ea0 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/api.test.tsx +++ b/x-pack/plugins/security_solution/public/cases/containers/api.test.tsx @@ -348,6 +348,7 @@ describe('Case Configuration API', () => { method: 'PATCH', body: JSON.stringify({ comment: 'updated comment', + type: CommentType.user, id: basicCase.comments[0].id, version: basicCase.comments[0].version, }), @@ -404,7 +405,7 @@ describe('Case Configuration API', () => { }); const data = { comment: 'comment', - type: CommentType.user, + type: CommentType.user as const, }; test('check url, method, signal', async () => { diff --git a/x-pack/plugins/security_solution/public/cases/containers/api.ts b/x-pack/plugins/security_solution/public/cases/containers/api.ts index 83ee10e9b45a82..6046c3716b3b5b 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/api.ts +++ b/x-pack/plugins/security_solution/public/cases/containers/api.ts @@ -11,13 +11,14 @@ import { CasePatchRequest, CasePostRequest, CasesStatusResponse, - CommentRequest, + CommentRequestUserType, User, CaseUserActionsResponse, CaseExternalServiceRequest, ServiceConnectorCaseParams, ServiceConnectorCaseResponse, ActionTypeExecutorResult, + CommentType, } from '../../../../case/common/api'; import { @@ -181,7 +182,7 @@ export const patchCasesStatus = async ( }; export const postComment = async ( - newComment: CommentRequest, + newComment: CommentRequestUserType, caseId: string, signal: AbortSignal ): Promise => { @@ -205,7 +206,12 @@ export const patchComment = async ( ): Promise => { const response = await KibanaServices.get().http.fetch(getCaseCommentsUrl(caseId), { method: 'PATCH', - body: JSON.stringify({ comment: commentUpdate, id: commentId, version }), + body: JSON.stringify({ + comment: commentUpdate, + type: CommentType.user, + id: commentId, + version, + }), signal, }); return convertToCamelCase(decodeCaseResponse(response)); diff --git a/x-pack/plugins/security_solution/public/cases/containers/types.ts b/x-pack/plugins/security_solution/public/cases/containers/types.ts index c2ddcce8b1d3ce..b9db356498a01b 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/types.ts +++ b/x-pack/plugins/security_solution/public/cases/containers/types.ts @@ -19,7 +19,7 @@ export interface Comment { createdAt: string; createdBy: ElasticUser; comment: string; - type: CommentType; + type: CommentType.user; pushedAt: string | null; pushedBy: string | null; updatedAt: string | null; diff --git a/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.test.tsx b/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.test.tsx index 773d4b8d1fe56d..39ee21f942cbd0 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.test.tsx +++ b/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.test.tsx @@ -17,7 +17,7 @@ describe('usePostComment', () => { const abortCtrl = new AbortController(); const samplePost = { comment: 'a comment', - type: CommentType.user, + type: CommentType.user as const, }; const updateCaseCallback = jest.fn(); beforeEach(() => { diff --git a/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.tsx b/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.tsx index e6cb8a9c3d150e..cd3827a2887fb6 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.tsx +++ b/x-pack/plugins/security_solution/public/cases/containers/use_post_comment.tsx @@ -6,7 +6,7 @@ import { useReducer, useCallback } from 'react'; -import { CommentRequest } from '../../../../case/common/api'; +import { CommentRequestUserType } from '../../../../case/common/api'; import { errorToToaster, useStateToaster } from '../../common/components/toasters'; import { postComment } from './api'; @@ -42,7 +42,7 @@ const dataFetchReducer = (state: NewCommentState, action: Action): NewCommentSta }; export interface UsePostComment extends NewCommentState { - postComment: (data: CommentRequest, updateCase: (newCase: Case) => void) => void; + postComment: (data: CommentRequestUserType, updateCase: (newCase: Case) => void) => void; } export const usePostComment = (caseId: string): UsePostComment => { @@ -53,7 +53,7 @@ export const usePostComment = (caseId: string): UsePostComment => { const [, dispatchToaster] = useStateToaster(); const postMyComment = useCallback( - async (data: CommentRequest, updateCase: (newCase: Case) => void) => { + async (data: CommentRequestUserType, updateCase: (newCase: Case) => void) => { let cancel = false; const abortCtrl = new AbortController(); diff --git a/x-pack/test/case_api_integration/basic/tests/cases/comments/delete_comment.ts b/x-pack/test/case_api_integration/basic/tests/cases/comments/delete_comment.ts index 5fb6f21c51c95a..6ab29ffa09e130 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/comments/delete_comment.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/comments/delete_comment.ts @@ -8,7 +8,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../../plugins/case/common/constants'; -import { postCaseReq, postCommentReq } from '../../../../common/lib/mock'; +import { postCaseReq, postCommentUserReq } from '../../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, deleteComments } from '../../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -33,7 +33,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body: comment } = await supertest @@ -55,7 +55,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body } = await supertest diff --git a/x-pack/test/case_api_integration/basic/tests/cases/comments/find_comments.ts b/x-pack/test/case_api_integration/basic/tests/cases/comments/find_comments.ts index c67eda1d3a16b6..180fc62d3d39ab 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/comments/find_comments.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/comments/find_comments.ts @@ -8,7 +8,8 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../../plugins/case/common/constants'; -import { postCaseReq, postCommentReq } from '../../../../common/lib/mock'; +import { CommentType } from '../../../../../../plugins/case/common/api'; +import { postCaseReq, postCommentUserReq } from '../../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, deleteComments } from '../../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -34,13 +35,13 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body: caseComments } = await supertest @@ -63,13 +64,13 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send({ comment: 'unique', type: 'user' }) + .send({ comment: 'unique', type: CommentType.user }) .expect(200); const { body: caseComments } = await supertest @@ -91,7 +92,7 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); await supertest diff --git a/x-pack/test/case_api_integration/basic/tests/cases/comments/get_comment.ts b/x-pack/test/case_api_integration/basic/tests/cases/comments/get_comment.ts index 9c3a85e99c29d4..e77405f3cd49b0 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/comments/get_comment.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/comments/get_comment.ts @@ -8,7 +8,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../../plugins/case/common/constants'; -import { postCaseReq, postCommentReq } from '../../../../common/lib/mock'; +import { postCaseReq, postCommentUserReq } from '../../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, deleteComments } from '../../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -33,7 +33,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body: comment } = await supertest diff --git a/x-pack/test/case_api_integration/basic/tests/cases/comments/patch_comment.ts b/x-pack/test/case_api_integration/basic/tests/cases/comments/patch_comment.ts index 3176841b009d40..ca24f0d2e32c59 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/comments/patch_comment.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/comments/patch_comment.ts @@ -4,11 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ +import { omit } from 'lodash/fp'; import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../../plugins/case/common/constants'; -import { defaultUser, postCaseReq, postCommentReq } from '../../../../common/lib/mock'; +import { CommentType } from '../../../../../../plugins/case/common/api'; +import { + defaultUser, + postCaseReq, + postCommentUserReq, + postCommentAlertReq, +} from '../../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, deleteComments } from '../../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -33,7 +40,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const newComment = 'Well I decided to update my comment. So what? Deal with it.'; @@ -44,10 +51,43 @@ export default ({ getService }: FtrProviderContext): void => { id: patchedCase.comments[0].id, version: patchedCase.comments[0].version, comment: newComment, + type: CommentType.user, }) .expect(200); expect(body.comments[0].comment).to.eql(newComment); + expect(body.comments[0].type).to.eql('user'); + expect(body.updated_by).to.eql(defaultUser); + }); + + it('should patch an alert', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const { body: patchedCase } = await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(postCommentAlertReq) + .expect(200); + + const { body } = await supertest + .patch(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + id: patchedCase.comments[0].id, + version: patchedCase.comments[0].version, + type: CommentType.alert, + alertId: 'new-id', + index: postCommentAlertReq.index, + }) + .expect(200); + + expect(body.comments[0].alertId).to.eql('new-id'); + expect(body.comments[0].index).to.eql(postCommentAlertReq.index); + expect(body.comments[0].type).to.eql('alert'); expect(body.updated_by).to.eql(defaultUser); }); @@ -64,6 +104,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ id: 'id', version: 'version', + type: CommentType.user, comment: 'comment', }) .expect(404); @@ -76,12 +117,39 @@ export default ({ getService }: FtrProviderContext): void => { .send({ id: 'id', version: 'version', + type: CommentType.user, comment: 'comment', }) .expect(404); }); - it('unhappy path - 400s when patch body is bad', async () => { + it('unhappy path - 400s when trying to change comment type', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const { body: patchedCase } = await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(postCommentUserReq) + .expect(200); + + await supertest + .patch(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + id: patchedCase.comments[0].id, + version: patchedCase.comments[0].version, + type: CommentType.alert, + alertId: 'test-id', + index: 'test-index', + }) + .expect(400); + }); + + it('unhappy path - 400s when missing attributes for type user', async () => { const { body: postedCase } = await supertest .post(CASES_URL) .set('kbn-xsrf', 'true') @@ -91,7 +159,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); await supertest @@ -100,11 +168,100 @@ export default ({ getService }: FtrProviderContext): void => { .send({ id: patchedCase.comments[0].id, version: patchedCase.comments[0].version, - comment: true, }) .expect(400); }); + it('unhappy path - 400s when adding excess attributes for type user', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const { body: patchedCase } = await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(postCommentUserReq) + .expect(200); + + for (const attribute of ['alertId', 'index']) { + await supertest + .patch(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + id: patchedCase.comments[0].id, + version: patchedCase.comments[0].version, + comment: 'a comment', + type: CommentType.user, + [attribute]: attribute, + }) + .expect(400); + } + }); + + it('unhappy path - 400s when missing attributes for type alert', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const { body: patchedCase } = await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(postCommentUserReq) + .expect(200); + + const allRequestAttributes = { + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }; + + for (const attribute of ['alertId', 'index']) { + const requestAttributes = omit(attribute, allRequestAttributes); + await supertest + .patch(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + id: patchedCase.comments[0].id, + version: patchedCase.comments[0].version, + ...requestAttributes, + }) + .expect(400); + } + }); + + it('unhappy path - 400s when adding excess attributes for type alert', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const { body: patchedCase } = await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(postCommentUserReq) + .expect(200); + + for (const attribute of ['comment']) { + await supertest + .patch(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + id: patchedCase.comments[0].id, + version: patchedCase.comments[0].version, + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + [attribute]: attribute, + }) + .expect(400); + } + }); + it('unhappy path - 409s when conflict', async () => { const { body: postedCase } = await supertest .post(CASES_URL) @@ -115,7 +272,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const newComment = 'Well I decided to update my comment. So what? Deal with it.'; @@ -125,6 +282,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ id: patchedCase.comments[0].id, version: 'version-mismatch', + type: CommentType.user, comment: newComment, }) .expect(409); diff --git a/x-pack/test/case_api_integration/basic/tests/cases/comments/post_comment.ts b/x-pack/test/case_api_integration/basic/tests/cases/comments/post_comment.ts index 0c7ab52abf8c87..d26e31394b9f56 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/comments/post_comment.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/comments/post_comment.ts @@ -4,11 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ +import { omit } from 'lodash/fp'; import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../../plugins/case/common/constants'; -import { defaultUser, postCaseReq, postCommentReq } from '../../../../common/lib/mock'; +import { CommentType } from '../../../../../../plugins/case/common/api'; +import { + defaultUser, + postCaseReq, + postCommentUserReq, + postCommentAlertReq, +} from '../../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, deleteComments } from '../../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -33,14 +40,50 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); - expect(patchedCase.comments[0].comment).to.eql(postCommentReq.comment); + expect(patchedCase.comments[0].type).to.eql(postCommentUserReq.type); + expect(patchedCase.comments[0].comment).to.eql(postCommentUserReq.comment); expect(patchedCase.updated_by).to.eql(defaultUser); }); - it('unhappy path - 400s when post body is bad', async () => { + it('should post an alert', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const { body: patchedCase } = await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(postCommentAlertReq) + .expect(200); + + expect(patchedCase.comments[0].type).to.eql(postCommentAlertReq.type); + expect(patchedCase.comments[0].alertId).to.eql(postCommentAlertReq.alertId); + expect(patchedCase.comments[0].index).to.eql(postCommentAlertReq.index); + expect(patchedCase.updated_by).to.eql(defaultUser); + }); + + it('unhappy path - 400s when type is missing', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + bad: 'comment', + }) + .expect(400); + }); + + it('unhappy path - 400s when missing attributes for type user', async () => { const { body: postedCase } = await supertest .post(CASES_URL) .set('kbn-xsrf', 'true') @@ -50,6 +93,74 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') + .send({ type: CommentType.user }) + .expect(400); + }); + + it('unhappy path - 400s when adding excess attributes for type user', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + for (const attribute of ['alertId', 'index']) { + await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ type: CommentType.user, [attribute]: attribute, comment: 'a comment' }) + .expect(400); + } + }); + + it('unhappy path - 400s when missing attributes for type alert', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const allRequestAttributes = { + type: CommentType.alert, + index: 'test-index', + alertId: 'test-id', + }; + + for (const attribute of ['alertId', 'index']) { + const requestAttributes = omit(attribute, allRequestAttributes); + await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send(requestAttributes) + .expect(400); + } + }); + + it('unhappy path - 400s when adding excess attributes for type alert', async () => { + const { body: postedCase } = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + for (const attribute of ['comment']) { + await supertest + .post(`${CASES_URL}/${postedCase.id}/comments`) + .set('kbn-xsrf', 'true') + .send({ + type: CommentType.alert, + [attribute]: attribute, + alertId: 'test-id', + index: 'test-index', + }) + .expect(400); + } + }); + + it('unhappy path - 400s when case is missing', async () => { + await supertest + .post(`${CASES_URL}/not-exists/comments`) + .set('kbn-xsrf', 'true') .send({ bad: 'comment', }) diff --git a/x-pack/test/case_api_integration/basic/tests/cases/delete_cases.ts b/x-pack/test/case_api_integration/basic/tests/cases/delete_cases.ts index 73d17b985216af..ac64818fe629e1 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/delete_cases.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/delete_cases.ts @@ -8,7 +8,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../plugins/case/common/constants'; -import { postCaseReq, postCommentReq } from '../../../common/lib/mock'; +import { postCaseReq, postCommentUserReq } from '../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, deleteComments } from '../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -49,7 +49,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); await supertest diff --git a/x-pack/test/case_api_integration/basic/tests/cases/find_cases.ts b/x-pack/test/case_api_integration/basic/tests/cases/find_cases.ts index 17814868fecc09..b119c71664f593 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/find_cases.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/find_cases.ts @@ -8,7 +8,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../plugins/case/common/constants'; -import { postCaseReq, postCommentReq, findCasesResp } from '../../../common/lib/mock'; +import { postCaseReq, postCommentUserReq, findCasesResp } from '../../../common/lib/mock'; import { deleteCases, deleteComments, deleteCasesUserActions } from '../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -98,13 +98,13 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body } = await supertest diff --git a/x-pack/test/case_api_integration/basic/tests/cases/push_case.ts b/x-pack/test/case_api_integration/basic/tests/cases/push_case.ts index 80cf2c8199807d..3cf0d6892377ef 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/push_case.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/push_case.ts @@ -9,7 +9,7 @@ import { FtrProviderContext } from '../../../common/ftr_provider_context'; import { ObjectRemover as ActionsRemover } from '../../../../alerting_api_integration/common/lib'; import { CASE_CONFIGURE_URL, CASES_URL } from '../../../../../plugins/case/common/constants'; -import { postCaseReq, defaultUser, postCommentReq } from '../../../common/lib/mock'; +import { postCaseReq, defaultUser, postCommentUserReq } from '../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, @@ -130,7 +130,7 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body } = await supertest diff --git a/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts b/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts index 92ef544ee9b379..6949052df47030 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts @@ -8,7 +8,8 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { CASE_CONFIGURE_URL, CASES_URL } from '../../../../../../plugins/case/common/constants'; -import { defaultUser, postCaseReq, postCommentReq } from '../../../../common/lib/mock'; +import { CommentType } from '../../../../../../plugins/case/common/api'; +import { defaultUser, postCaseReq, postCommentUserReq } from '../../../../common/lib/mock'; import { deleteCases, deleteCasesUserActions, @@ -251,7 +252,7 @@ export default ({ getService }: FtrProviderContext): void => { await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const { body } = await supertest @@ -264,7 +265,7 @@ export default ({ getService }: FtrProviderContext): void => { expect(body[1].action_field).to.eql(['comment']); expect(body[1].action).to.eql('create'); expect(body[1].old_value).to.eql(null); - expect(body[1].new_value).to.eql(postCommentReq.comment); + expect(body[1].new_value).to.eql(JSON.stringify(postCommentUserReq)); }); it(`on update comment, user action: 'update' should be called with actionFields: ['comments']`, async () => { @@ -277,7 +278,7 @@ export default ({ getService }: FtrProviderContext): void => { const { body: patchedCase } = await supertest .post(`${CASES_URL}/${postedCase.id}/comments`) .set('kbn-xsrf', 'true') - .send(postCommentReq) + .send(postCommentUserReq) .expect(200); const newComment = 'Well I decided to update my comment. So what? Deal with it.'; @@ -285,6 +286,7 @@ export default ({ getService }: FtrProviderContext): void => { id: patchedCase.comments[0].id, version: patchedCase.comments[0].version, comment: newComment, + type: CommentType.user, }); const { body } = await supertest @@ -296,8 +298,13 @@ export default ({ getService }: FtrProviderContext): void => { expect(body.length).to.eql(3); expect(body[2].action_field).to.eql(['comment']); expect(body[2].action).to.eql('update'); - expect(body[2].old_value).to.eql(postCommentReq.comment); - expect(body[2].new_value).to.eql(newComment); + expect(body[2].old_value).to.eql(JSON.stringify(postCommentUserReq)); + expect(body[2].new_value).to.eql( + JSON.stringify({ + comment: newComment, + type: CommentType.user, + }) + ); }); it(`on new push to service, user action: 'push-to-service' should be called with actionFields: ['pushed']`, async () => { diff --git a/x-pack/test/case_api_integration/basic/tests/connectors/case.ts b/x-pack/test/case_api_integration/basic/tests/connectors/case.ts index 7a351d09b5b9f4..9a45dd541bb562 100644 --- a/x-pack/test/case_api_integration/basic/tests/connectors/case.ts +++ b/x-pack/test/case_api_integration/basic/tests/connectors/case.ts @@ -4,10 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ +import { omit } from 'lodash/fp'; import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; import { CASES_URL } from '../../../../../plugins/case/common/constants'; +import { CommentType } from '../../../../../plugins/case/common/api'; import { postCaseReq, postCaseResp, @@ -616,9 +618,9 @@ export default ({ getService }: FtrProviderContext): void => { createdActionId = createdAction.id; const params = { - subAction: 'update', + subAction: 'addComment', subActionParams: { - comment: { comment: 'a comment', type: 'user' }, + comment: { comment: 'a comment', type: CommentType.user }, }, }; @@ -632,12 +634,12 @@ export default ({ getService }: FtrProviderContext): void => { status: 'error', actionId: createdActionId, message: - 'error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subActionParams.id]: expected value of type [string] but got [undefined]\n- [2.subAction]: expected value to equal [addComment]', + 'error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subAction]: expected value to equal [update]\n- [2.subActionParams.caseId]: expected value of type [string] but got [undefined]', retry: false, }); }); - it('should respond with a 400 Bad Request when adding a comment to a case without comment', async () => { + it('should respond with a 400 Bad Request when missing attributes of type user', async () => { const { body: createdAction } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'foo') @@ -650,7 +652,7 @@ export default ({ getService }: FtrProviderContext): void => { createdActionId = createdAction.id; const params = { - subAction: 'update', + subAction: 'addComment', subActionParams: { caseId: '123', }, @@ -666,12 +668,143 @@ export default ({ getService }: FtrProviderContext): void => { status: 'error', actionId: createdActionId, message: - 'error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subActionParams.id]: expected value of type [string] but got [undefined]\n- [2.subAction]: expected value to equal [addComment]', + 'error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subAction]: expected value to equal [update]\n- [2.subActionParams.comment]: expected at least one defined value but got [undefined]', retry: false, }); }); - it('should respond with a 400 Bad Request when adding a comment to a case without comment type', async () => { + it('should respond with a 400 Bad Request when missing attributes of type alert', async () => { + const { body: createdAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'A case connector', + actionTypeId: '.case', + config: {}, + }) + .expect(200); + + createdActionId = createdAction.id; + const comment = { alertId: 'test-id', index: 'test-index', type: CommentType.alert }; + const params = { + subAction: 'addComment', + subActionParams: { + caseId: '123', + comment, + }, + }; + + for (const attribute of ['alertId', 'index']) { + const requestAttributes = omit(attribute, comment); + const caseConnector = await supertest + .post(`/api/actions/action/${createdActionId}/_execute`) + .set('kbn-xsrf', 'foo') + .send({ + params: { + ...params, + subActionParams: { ...params.subActionParams, comment: requestAttributes }, + }, + }) + .expect(200); + + expect(caseConnector.body).to.eql({ + status: 'error', + actionId: createdActionId, + message: `error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subAction]: expected value to equal [update]\n- [2.subActionParams.comment]: types that failed validation:\n - [subActionParams.comment.0.type]: expected value to equal [user]\n - [subActionParams.comment.1.${attribute}]: expected value of type [string] but got [undefined]`, + retry: false, + }); + } + }); + + it('should respond with a 400 Bad Request when adding excess attributes for type user', async () => { + const { body: createdAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'A case connector', + actionTypeId: '.case', + config: {}, + }) + .expect(200); + + createdActionId = createdAction.id; + const params = { + subAction: 'addComment', + subActionParams: { + caseId: '123', + comment: { comment: 'a comment', type: CommentType.user }, + }, + }; + + for (const attribute of ['alertId', 'index']) { + const caseConnector = await supertest + .post(`/api/actions/action/${createdActionId}/_execute`) + .set('kbn-xsrf', 'foo') + .send({ + params: { + ...params, + subActionParams: { + ...params.subActionParams, + comment: { ...params.subActionParams.comment, [attribute]: attribute }, + }, + }, + }) + .expect(200); + + expect(caseConnector.body).to.eql({ + status: 'error', + actionId: createdActionId, + message: `error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subAction]: expected value to equal [update]\n- [2.subActionParams.comment]: types that failed validation:\n - [subActionParams.comment.0.${attribute}]: definition for this key is missing\n - [subActionParams.comment.1.type]: expected value to equal [alert]`, + retry: false, + }); + } + }); + + it('should respond with a 400 Bad Request when adding excess attributes for type alert', async () => { + const { body: createdAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'A case connector', + actionTypeId: '.case', + config: {}, + }) + .expect(200); + + createdActionId = createdAction.id; + const params = { + subAction: 'addComment', + subActionParams: { + caseId: '123', + comment: { alertId: 'test-id', index: 'test-index', type: CommentType.alert }, + }, + }; + + for (const attribute of ['comment']) { + const caseConnector = await supertest + .post(`/api/actions/action/${createdActionId}/_execute`) + .set('kbn-xsrf', 'foo') + .send({ + params: { + ...params, + subActionParams: { + ...params.subActionParams, + comment: { ...params.subActionParams.comment, [attribute]: attribute }, + }, + }, + }) + .expect(200); + + expect(caseConnector.body).to.eql({ + status: 'error', + actionId: createdActionId, + message: `error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [create]\n- [1.subAction]: expected value to equal [update]\n- [2.subActionParams.comment]: types that failed validation:\n - [subActionParams.comment.0.type]: expected value to equal [user]\n - [subActionParams.comment.1.${attribute}]: definition for this key is missing`, + retry: false, + }); + } + }); + + it('should respond with a 400 Bad Request when adding a comment to a case without type', async () => { const { body: createdAction } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'foo') @@ -706,7 +839,60 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should add a comment', async () => { + it('should add a comment of type user', async () => { + const { body: createdAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'A case connector', + actionTypeId: '.case', + config: {}, + }) + .expect(200); + + createdActionId = createdAction.id; + + const caseRes = await supertest + .post(CASES_URL) + .set('kbn-xsrf', 'true') + .send(postCaseReq) + .expect(200); + + const params = { + subAction: 'addComment', + subActionParams: { + caseId: caseRes.body.id, + comment: { comment: 'a comment', type: CommentType.user }, + }, + }; + + await supertest + .post(`/api/actions/action/${createdActionId}/_execute`) + .set('kbn-xsrf', 'foo') + .send({ params }) + .expect(200); + + const { body } = await supertest + .get(`${CASES_URL}/${caseRes.body.id}`) + .set('kbn-xsrf', 'true') + .send() + .expect(200); + + const data = removeServerGeneratedPropertiesFromCase(body); + const comments = removeServerGeneratedPropertiesFromComments(data.comments ?? []); + expect({ ...data, comments }).to.eql({ + ...postCaseResp(caseRes.body.id), + comments, + totalComment: 1, + updated_by: { + email: null, + full_name: null, + username: null, + }, + }); + }); + + it('should add a comment of type alert', async () => { const { body: createdAction } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'foo') @@ -729,7 +915,7 @@ export default ({ getService }: FtrProviderContext): void => { subAction: 'addComment', subActionParams: { caseId: caseRes.body.id, - comment: { comment: 'a comment', type: 'user' }, + comment: { alertId: 'test-id', index: 'test-index', type: CommentType.alert }, }, }; diff --git a/x-pack/test/case_api_integration/common/lib/mock.ts b/x-pack/test/case_api_integration/common/lib/mock.ts index d2262c684dc6da..a1e7f9a7fa89e1 100644 --- a/x-pack/test/case_api_integration/common/lib/mock.ts +++ b/x-pack/test/case_api_integration/common/lib/mock.ts @@ -10,6 +10,9 @@ import { CasesFindResponse, CommentResponse, ConnectorTypes, + CommentRequestUserType, + CommentRequestAlertType, + CommentType, } from '../../../../plugins/case/common/api'; export const defaultUser = { email: null, full_name: null, username: 'elastic' }; export const postCaseReq: CasePostRequest = { @@ -24,9 +27,15 @@ export const postCaseReq: CasePostRequest = { }, }; -export const postCommentReq: { comment: string; type: string } = { +export const postCommentUserReq: CommentRequestUserType = { comment: 'This is a cool comment', - type: 'user', + type: CommentType.user, +}; + +export const postCommentAlertReq: CommentRequestAlertType = { + alertId: 'test-id', + index: 'test-index', + type: CommentType.alert, }; export const postCaseResp = (