From 94ed783cae758b7fdb8e7a10e3bed554d2bdb0d6 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Mon, 27 Jul 2020 18:19:16 -0400 Subject: [PATCH] [Security Solution][Exceptions] - Update exception item comments to include id (#73129) ## Summary This PR is somewhat of an intermediary step. Comments on exception list items are denormalized. We initially decided that we would not add `uuid` to comments, but found that it is in fact necessary. This is intermediary in the sense that what we ideally want to have is a dedicated `comments` CRUD route. Also just note that I added a callout for when a version conflict occurs (ie: exception item was updated by someone else while a user is editing the same item). With this PR users are able to: - Create comments when creating exception list items - Add new comments on exception item update Users will currently be blocked from: - Deleting comments - Updating comments - Updating exception item if version conflict is found --- x-pack/plugins/lists/common/constants.mock.ts | 1 + .../create_endpoint_list_item_schema.test.ts | 36 +- .../create_exception_list_item_schema.test.ts | 36 +- ...ate_exception_list_item_validation.test.ts | 43 ++ .../update_exception_list_item_validation.ts | 40 ++ .../{comments.mock.ts => comment.mock.ts} | 7 +- .../{comments.test.ts => comment.test.ts} | 109 +++-- .../schemas/types/{comments.ts => comment.ts} | 23 +- ...omments.mock.ts => create_comment.mock.ts} | 4 +- ...omments.test.ts => create_comment.test.ts} | 50 +-- .../{create_comments.ts => create_comment.ts} | 11 +- .../types/default_comments_array.test.ts | 21 +- .../schemas/types/default_comments_array.ts | 6 +- .../default_create_comments_array.test.ts | 30 +- .../types/default_create_comments_array.ts | 6 +- .../default_update_comments_array.test.ts | 23 +- .../types/default_update_comments_array.ts | 2 +- .../lists/common/schemas/types/index.ts | 6 +- ...omments.mock.ts => update_comment.mock.ts} | 15 +- .../schemas/types/update_comment.test.ts | 150 +++++++ .../{update_comments.ts => update_comment.ts} | 20 +- .../schemas/types/update_comments.test.ts | 108 ----- x-pack/plugins/lists/common/shared_exports.ts | 5 +- .../update_exception_list_item_route.ts | 6 + .../server/saved_objects/exception_list.ts | 3 + .../updates/simple_update_item.json | 25 +- .../create_exception_list_item.ts | 5 +- .../services/exception_lists/utils.test.ts | 390 ++---------------- .../server/services/exception_lists/utils.ts | 115 +----- .../common/shared_imports.ts | 5 +- .../exceptions/add_exception_comments.tsx | 4 +- .../exceptions/add_exception_modal/index.tsx | 4 +- .../components/exceptions/builder/index.tsx | 2 +- .../exceptions/edit_exception_modal/index.tsx | 40 +- .../edit_exception_modal/translations.ts | 15 + .../components/exceptions/helpers.test.tsx | 55 ++- .../common/components/exceptions/helpers.tsx | 55 ++- .../exception_item/exception_details.test.tsx | 2 +- .../viewer/exception_item/index.stories.tsx | 2 +- .../viewer/exception_item/index.test.tsx | 2 +- .../components/exceptions/viewer/index.tsx | 3 +- 41 files changed, 702 insertions(+), 783 deletions(-) create mode 100644 x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.test.ts create mode 100644 x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.ts rename x-pack/plugins/lists/common/schemas/types/{comments.mock.ts => comment.mock.ts} (71%) rename x-pack/plugins/lists/common/schemas/types/{comments.test.ts => comment.test.ts} (56%) rename x-pack/plugins/lists/common/schemas/types/{comments.ts => comment.ts} (56%) rename x-pack/plugins/lists/common/schemas/types/{create_comments.mock.ts => create_comment.mock.ts} (73%) rename x-pack/plugins/lists/common/schemas/types/{create_comments.test.ts => create_comment.test.ts} (72%) rename x-pack/plugins/lists/common/schemas/types/{create_comments.ts => create_comment.ts} (64%) rename x-pack/plugins/lists/common/schemas/types/{update_comments.mock.ts => update_comment.mock.ts} (54%) create mode 100644 x-pack/plugins/lists/common/schemas/types/update_comment.test.ts rename x-pack/plugins/lists/common/schemas/types/{update_comments.ts => update_comment.ts} (58%) delete mode 100644 x-pack/plugins/lists/common/schemas/types/update_comments.test.ts diff --git a/x-pack/plugins/lists/common/constants.mock.ts b/x-pack/plugins/lists/common/constants.mock.ts index 30f219c3ec101..22706890e2020 100644 --- a/x-pack/plugins/lists/common/constants.mock.ts +++ b/x-pack/plugins/lists/common/constants.mock.ts @@ -6,6 +6,7 @@ import { EntriesArray } from './schemas/types'; export const DATE_NOW = '2020-04-20T15:25:31.830Z'; +export const OLD_DATE_RELATIVE_TO_DATE_NOW = '2020-04-19T15:25:31.830Z'; export const USER = 'some user'; export const LIST_INDEX = '.lists'; export const LIST_ITEM_INDEX = '.items'; diff --git a/x-pack/plugins/lists/common/schemas/request/create_endpoint_list_item_schema.test.ts b/x-pack/plugins/lists/common/schemas/request/create_endpoint_list_item_schema.test.ts index 5de9fbb0d5b50..75e0410be610a 100644 --- a/x-pack/plugins/lists/common/schemas/request/create_endpoint_list_item_schema.test.ts +++ b/x-pack/plugins/lists/common/schemas/request/create_endpoint_list_item_schema.test.ts @@ -8,8 +8,8 @@ import { left } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; import { exactCheck, foldLeftRight, getPaths } from '../../siem_common_deps'; -import { getCreateCommentsArrayMock } from '../types/create_comments.mock'; -import { getCommentsMock } from '../types/comments.mock'; +import { getCreateCommentsArrayMock } from '../types/create_comment.mock'; +import { getCommentsMock } from '../types/comment.mock'; import { CommentsArray } from '../types'; import { @@ -19,7 +19,7 @@ import { import { getCreateEndpointListItemSchemaMock } from './create_endpoint_list_item_schema.mock'; describe('create_endpoint_list_item_schema', () => { - test('it should validate a typical list item request not counting the auto generated uuid', () => { + test('it should pass validation when supplied a typical list item request not counting the auto generated uuid', () => { const payload = getCreateEndpointListItemSchemaMock(); const decoded = createEndpointListItemSchema.decode(payload); const checked = exactCheck(payload, decoded); @@ -29,7 +29,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual(payload); }); - test('it should not validate an undefined for "description"', () => { + test('it should fail validation when supplied an undefined for "description"', () => { const payload = getCreateEndpointListItemSchemaMock(); delete payload.description; const decoded = createEndpointListItemSchema.decode(payload); @@ -41,7 +41,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate an undefined for "name"', () => { + test('it should fail validation when supplied an undefined for "name"', () => { const payload = getCreateEndpointListItemSchemaMock(); delete payload.name; const decoded = createEndpointListItemSchema.decode(payload); @@ -53,7 +53,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate an undefined for "type"', () => { + test('it should fail validation when supplied an undefined for "type"', () => { const payload = getCreateEndpointListItemSchemaMock(); delete payload.type; const decoded = createEndpointListItemSchema.decode(payload); @@ -65,7 +65,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate a "list_id" since it does not required one', () => { + test('it should fail validation when supplied a "list_id" since it does not required one', () => { const inputPayload: CreateEndpointListItemSchema & { list_id: string } = { ...getCreateEndpointListItemSchemaMock(), list_id: 'list-123', @@ -77,7 +77,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate a "namespace_type" since it does not required one', () => { + test('it should fail validation when supplied a "namespace_type" since it does not required one', () => { const inputPayload: CreateEndpointListItemSchema & { namespace_type: string } = { ...getCreateEndpointListItemSchemaMock(), namespace_type: 'single', @@ -89,7 +89,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should validate an undefined for "meta" but strip it out and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "meta" but strip it out and generate a correct body not counting the auto generated uuid', () => { const payload = getCreateEndpointListItemSchemaMock(); const outputPayload = getCreateEndpointListItemSchemaMock(); delete payload.meta; @@ -102,7 +102,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "comments" but return an array and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "comments" but return an array and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateEndpointListItemSchemaMock(); const outputPayload = getCreateEndpointListItemSchemaMock(); delete inputPayload.comments; @@ -115,7 +115,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate "comments" array', () => { + test('it should pass validation when supplied "comments" array', () => { const inputPayload = { ...getCreateEndpointListItemSchemaMock(), comments: getCreateCommentsArrayMock(), @@ -128,7 +128,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual(inputPayload); }); - test('it should NOT validate "comments" with "created_at" or "created_by" values', () => { + test('it should fail validation when supplied "comments" with "created_at", "created_by", or "id" values', () => { const inputPayload: Omit & { comments?: CommentsArray; } = { @@ -138,11 +138,11 @@ describe('create_endpoint_list_item_schema', () => { const decoded = createEndpointListItemSchema.decode(inputPayload); const checked = exactCheck(inputPayload, decoded); const message = pipe(checked, foldLeftRight); - expect(getPaths(left(message.errors))).toEqual(['invalid keys "created_at,created_by"']); + expect(getPaths(left(message.errors))).toEqual(['invalid keys "created_at,created_by,id"']); expect(message.schema).toEqual({}); }); - test('it should NOT validate an undefined for "entries"', () => { + test('it should fail validation when supplied an undefined for "entries"', () => { const inputPayload = getCreateEndpointListItemSchemaMock(); const outputPayload = getCreateEndpointListItemSchemaMock(); delete inputPayload.entries; @@ -157,7 +157,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should validate an undefined for "tags" but return an array and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "tags" but return an array and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateEndpointListItemSchemaMock(); const outputPayload = getCreateEndpointListItemSchemaMock(); delete inputPayload.tags; @@ -170,7 +170,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "_tags" but return an array and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "_tags" but return an array and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateEndpointListItemSchemaMock(); const outputPayload = getCreateEndpointListItemSchemaMock(); delete inputPayload._tags; @@ -183,7 +183,7 @@ describe('create_endpoint_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "item_id" and auto generate a uuid', () => { + test('it should pass validation when supplied an undefined for "item_id" and auto generate a uuid', () => { const inputPayload = getCreateEndpointListItemSchemaMock(); delete inputPayload.item_id; const decoded = createEndpointListItemSchema.decode(inputPayload); @@ -195,7 +195,7 @@ describe('create_endpoint_list_item_schema', () => { ); }); - test('it should validate an undefined for "item_id" and generate a correct body not counting the uuid', () => { + test('it should pass validation when supplied an undefined for "item_id" and generate a correct body not counting the uuid', () => { const inputPayload = getCreateEndpointListItemSchemaMock(); delete inputPayload.item_id; const decoded = createEndpointListItemSchema.decode(inputPayload); diff --git a/x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.test.ts b/x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.test.ts index 08f3966af08d9..cf4c1fea0306f 100644 --- a/x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.test.ts +++ b/x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.test.ts @@ -8,8 +8,8 @@ import { left } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; import { exactCheck, foldLeftRight, getPaths } from '../../siem_common_deps'; -import { getCreateCommentsArrayMock } from '../types/create_comments.mock'; -import { getCommentsMock } from '../types/comments.mock'; +import { getCreateCommentsArrayMock } from '../types/create_comment.mock'; +import { getCommentsMock } from '../types/comment.mock'; import { CommentsArray } from '../types'; import { @@ -19,7 +19,7 @@ import { import { getCreateExceptionListItemSchemaMock } from './create_exception_list_item_schema.mock'; describe('create_exception_list_item_schema', () => { - test('it should validate a typical exception list item request not counting the auto generated uuid', () => { + test('it should pass validation when supplied a typical exception list item request not counting the auto generated uuid', () => { const payload = getCreateExceptionListItemSchemaMock(); const decoded = createExceptionListItemSchema.decode(payload); const checked = exactCheck(payload, decoded); @@ -29,7 +29,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(payload); }); - test('it should not validate an undefined for "description"', () => { + test('it should fail validation when supplied an undefined for "description"', () => { const payload = getCreateExceptionListItemSchemaMock(); delete payload.description; const decoded = createExceptionListItemSchema.decode(payload); @@ -41,7 +41,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate an undefined for "name"', () => { + test('it should fail validation when supplied an undefined for "name"', () => { const payload = getCreateExceptionListItemSchemaMock(); delete payload.name; const decoded = createExceptionListItemSchema.decode(payload); @@ -53,7 +53,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate an undefined for "type"', () => { + test('it should fail validation when supplied an undefined for "type"', () => { const payload = getCreateExceptionListItemSchemaMock(); delete payload.type; const decoded = createExceptionListItemSchema.decode(payload); @@ -65,7 +65,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should not validate an undefined for "list_id"', () => { + test('it should fail validation when supplied an undefined for "list_id"', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.list_id; const decoded = createExceptionListItemSchema.decode(inputPayload); @@ -77,7 +77,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should validate an undefined for "meta" but strip it out and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "meta" but strip it out and generate a correct body not counting the auto generated uuid', () => { const payload = getCreateExceptionListItemSchemaMock(); const outputPayload = getCreateExceptionListItemSchemaMock(); delete payload.meta; @@ -90,7 +90,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "comments" but return an array and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "comments" but return an array and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); const outputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.comments; @@ -103,7 +103,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate "comments" array', () => { + test('it should pass validation when supplied "comments" array', () => { const inputPayload = { ...getCreateExceptionListItemSchemaMock(), comments: getCreateCommentsArrayMock(), @@ -116,7 +116,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(inputPayload); }); - test('it should NOT validate "comments" with "created_at" or "created_by" values', () => { + test('it should fail validation when supplied "comments" with "created_at" or "created_by" values', () => { const inputPayload: Omit & { comments?: CommentsArray; } = { @@ -126,11 +126,11 @@ describe('create_exception_list_item_schema', () => { const decoded = createExceptionListItemSchema.decode(inputPayload); const checked = exactCheck(inputPayload, decoded); const message = pipe(checked, foldLeftRight); - expect(getPaths(left(message.errors))).toEqual(['invalid keys "created_at,created_by"']); + expect(getPaths(left(message.errors))).toEqual(['invalid keys "created_at,created_by,id"']); expect(message.schema).toEqual({}); }); - test('it should NOT validate an undefined for "entries"', () => { + test('it should fail validation when supplied an undefined for "entries"', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); const outputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.entries; @@ -145,7 +145,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual({}); }); - test('it should validate an undefined for "namespace_type" but return enum "single" and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "namespace_type" but return enum "single" and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); const outputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.namespace_type; @@ -158,7 +158,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "tags" but return an array and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "tags" but return an array and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); const outputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.tags; @@ -171,7 +171,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "_tags" but return an array and generate a correct body not counting the auto generated uuid', () => { + test('it should pass validation when supplied an undefined for "_tags" but return an array and generate a correct body not counting the auto generated uuid', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); const outputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload._tags; @@ -184,7 +184,7 @@ describe('create_exception_list_item_schema', () => { expect(message.schema).toEqual(outputPayload); }); - test('it should validate an undefined for "item_id" and auto generate a uuid', () => { + test('it should pass validation when supplied an undefined for "item_id" and auto generate a uuid', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.item_id; const decoded = createExceptionListItemSchema.decode(inputPayload); @@ -196,7 +196,7 @@ describe('create_exception_list_item_schema', () => { ); }); - test('it should validate an undefined for "item_id" and generate a correct body not counting the uuid', () => { + test('it should pass validation when supplied an undefined for "item_id" and generate a correct body not counting the uuid', () => { const inputPayload = getCreateExceptionListItemSchemaMock(); delete inputPayload.item_id; const decoded = createExceptionListItemSchema.decode(inputPayload); diff --git a/x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.test.ts b/x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.test.ts new file mode 100644 index 0000000000000..3358582786cc7 --- /dev/null +++ b/x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.test.ts @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { getUpdateExceptionListItemSchemaMock } from './update_exception_list_item_schema.mock'; +import { validateComments } from './update_exception_list_item_validation'; + +describe('update_exception_list_item_validation', () => { + describe('#validateComments', () => { + test('it returns no errors if comments is undefined', () => { + const payload = getUpdateExceptionListItemSchemaMock(); + delete payload.comments; + const output = validateComments(payload); + + expect(output).toEqual([]); + }); + + test('it returns no errors if new comments are append only', () => { + const payload = getUpdateExceptionListItemSchemaMock(); + payload.comments = [ + { comment: 'Im an old comment', id: '1' }, + { comment: 'Im a new comment' }, + ]; + const output = validateComments(payload); + + expect(output).toEqual([]); + }); + + test('it returns error if comments are not append only', () => { + const payload = getUpdateExceptionListItemSchemaMock(); + payload.comments = [ + { comment: 'Im an old comment', id: '1' }, + { comment: 'Im a new comment modifying the order of existing comments' }, + { comment: 'Im an old comment', id: '2' }, + ]; + const output = validateComments(payload); + + expect(output).toEqual(['item "comments" are append only']); + }); + }); +}); diff --git a/x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.ts b/x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.ts new file mode 100644 index 0000000000000..5e44c4e9f73e7 --- /dev/null +++ b/x-pack/plugins/lists/common/schemas/request/update_exception_list_item_validation.ts @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { UpdateExceptionListItemSchema } from './update_exception_list_item_schema'; + +export const validateComments = (item: UpdateExceptionListItemSchema): string[] => { + if (item.comments == null) { + return []; + } + + const [appendOnly] = item.comments.reduce( + (acc, comment) => { + const [, hasNewComments] = acc; + if (comment.id == null) { + return [true, true]; + } + + if (hasNewComments && comment.id != null) { + return [false, true]; + } + + return acc; + }, + [true, false] + ); + if (!appendOnly) { + return ['item "comments" are append only']; + } else { + return []; + } +}; + +export const updateExceptionListItemValidate = ( + schema: UpdateExceptionListItemSchema +): string[] => { + return [...validateComments(schema)]; +}; diff --git a/x-pack/plugins/lists/common/schemas/types/comments.mock.ts b/x-pack/plugins/lists/common/schemas/types/comment.mock.ts similarity index 71% rename from x-pack/plugins/lists/common/schemas/types/comments.mock.ts rename to x-pack/plugins/lists/common/schemas/types/comment.mock.ts index 9e56ac292f8b5..213259b3cce29 100644 --- a/x-pack/plugins/lists/common/schemas/types/comments.mock.ts +++ b/x-pack/plugins/lists/common/schemas/types/comment.mock.ts @@ -4,14 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ -import { DATE_NOW, USER } from '../../constants.mock'; +import { DATE_NOW, ID, USER } from '../../constants.mock'; -import { Comments, CommentsArray } from './comments'; +import { Comment, CommentsArray } from './comment'; -export const getCommentsMock = (): Comments => ({ +export const getCommentsMock = (): Comment => ({ comment: 'some old comment', created_at: DATE_NOW, created_by: USER, + id: ID, }); export const getCommentsArrayMock = (): CommentsArray => [getCommentsMock(), getCommentsMock()]; diff --git a/x-pack/plugins/lists/common/schemas/types/comments.test.ts b/x-pack/plugins/lists/common/schemas/types/comment.test.ts similarity index 56% rename from x-pack/plugins/lists/common/schemas/types/comments.test.ts rename to x-pack/plugins/lists/common/schemas/types/comment.test.ts index 29bfde03abcc8..c7c945277f756 100644 --- a/x-pack/plugins/lists/common/schemas/types/comments.test.ts +++ b/x-pack/plugins/lists/common/schemas/types/comment.test.ts @@ -10,56 +10,79 @@ import { left } from 'fp-ts/lib/Either'; import { DATE_NOW } from '../../constants.mock'; import { foldLeftRight, getPaths } from '../../siem_common_deps'; -import { getCommentsArrayMock, getCommentsMock } from './comments.mock'; +import { getCommentsArrayMock, getCommentsMock } from './comment.mock'; import { - Comments, + Comment, CommentsArray, CommentsArrayOrUndefined, - comments, + comment, commentsArray, commentsArrayOrUndefined, -} from './comments'; +} from './comment'; -describe('Comments', () => { - describe('comments', () => { - test('it should validate a comments', () => { +describe('Comment', () => { + describe('comment', () => { + test('it fails validation when "id" is undefined', () => { + const payload = { ...getCommentsMock(), id: undefined }; + const decoded = comment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "id"', + ]); + expect(message.schema).toEqual({}); + }); + + test('it passes validation with a typical comment', () => { const payload = getCommentsMock(); - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([]); expect(message.schema).toEqual(payload); }); - test('it should validate with "updated_at" and "updated_by"', () => { + test('it passes validation with "updated_at" and "updated_by" fields included', () => { const payload = getCommentsMock(); payload.updated_at = DATE_NOW; payload.updated_by = 'someone'; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([]); expect(message.schema).toEqual(payload); }); - test('it should not validate when undefined', () => { + test('it fails validation when undefined', () => { const payload = undefined; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "undefined" supplied to "({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)"', - 'Invalid value "undefined" supplied to "({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)"', + 'Invalid value "undefined" supplied to "({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)"', + 'Invalid value "undefined" supplied to "({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)"', ]); expect(message.schema).toEqual({}); }); - test('it should not validate when "comment" is not a string', () => { - const payload: Omit & { comment: string[] } = { + test('it fails validation when "comment" is an empty string', () => { + const payload: Omit & { comment: string } = { + ...getCommentsMock(), + comment: '', + }; + const decoded = comment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual(['Invalid value "" supplied to "comment"']); + expect(message.schema).toEqual({}); + }); + + test('it fails validation when "comment" is not a string', () => { + const payload: Omit & { comment: string[] } = { ...getCommentsMock(), comment: ['some value'], }; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ @@ -68,12 +91,12 @@ describe('Comments', () => { expect(message.schema).toEqual({}); }); - test('it should not validate when "created_at" is not a string', () => { - const payload: Omit & { created_at: number } = { + test('it fails validation when "created_at" is not a string', () => { + const payload: Omit & { created_at: number } = { ...getCommentsMock(), created_at: 1, }; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ @@ -82,12 +105,12 @@ describe('Comments', () => { expect(message.schema).toEqual({}); }); - test('it should not validate when "created_by" is not a string', () => { - const payload: Omit & { created_by: number } = { + test('it fails validation when "created_by" is not a string', () => { + const payload: Omit & { created_by: number } = { ...getCommentsMock(), created_by: 1, }; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ @@ -96,12 +119,12 @@ describe('Comments', () => { expect(message.schema).toEqual({}); }); - test('it should not validate when "updated_at" is not a string', () => { - const payload: Omit & { updated_at: number } = { + test('it fails validation when "updated_at" is not a string', () => { + const payload: Omit & { updated_at: number } = { ...getCommentsMock(), updated_at: 1, }; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ @@ -110,12 +133,12 @@ describe('Comments', () => { expect(message.schema).toEqual({}); }); - test('it should not validate when "updated_by" is not a string', () => { - const payload: Omit & { updated_by: number } = { + test('it fails validation when "updated_by" is not a string', () => { + const payload: Omit & { updated_by: number } = { ...getCommentsMock(), updated_by: 1, }; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ @@ -125,11 +148,11 @@ describe('Comments', () => { }); test('it should strip out extra keys', () => { - const payload: Comments & { + const payload: Comment & { extraKey?: string; } = getCommentsMock(); payload.extraKey = 'some value'; - const decoded = comments.decode(payload); + const decoded = comment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([]); @@ -138,7 +161,7 @@ describe('Comments', () => { }); describe('commentsArray', () => { - test('it should validate an array of comments', () => { + test('it passes validation an array of Comment', () => { const payload = getCommentsArrayMock(); const decoded = commentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -147,7 +170,7 @@ describe('Comments', () => { expect(message.schema).toEqual(payload); }); - test('it should validate when a comments includes "updated_at" and "updated_by"', () => { + test('it passes validation when a Comment includes "updated_at" and "updated_by"', () => { const commentsPayload = getCommentsMock(); commentsPayload.updated_at = DATE_NOW; commentsPayload.updated_by = 'someone'; @@ -159,32 +182,32 @@ describe('Comments', () => { expect(message.schema).toEqual(payload); }); - test('it should not validate when undefined', () => { + test('it fails validation when undefined', () => { const payload = undefined; const decoded = commentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "undefined" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "undefined" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', ]); expect(message.schema).toEqual({}); }); - test('it should not validate when array includes non comments types', () => { + test('it fails validation when array includes non Comment types', () => { const payload = ([1] as unknown) as CommentsArray; const decoded = commentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', - 'Invalid value "1" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', ]); expect(message.schema).toEqual({}); }); }); describe('commentsArrayOrUndefined', () => { - test('it should validate an array of comments', () => { + test('it passes validation an array of Comment', () => { const payload = getCommentsArrayMock(); const decoded = commentsArrayOrUndefined.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -193,7 +216,7 @@ describe('Comments', () => { expect(message.schema).toEqual(payload); }); - test('it should validate when undefined', () => { + test('it passes validation when undefined', () => { const payload = undefined; const decoded = commentsArrayOrUndefined.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -202,14 +225,14 @@ describe('Comments', () => { expect(message.schema).toEqual(payload); }); - test('it should not validate when array includes non comments types', () => { + test('it fails validation when array includes non Comment types', () => { const payload = ([1] as unknown) as CommentsArrayOrUndefined; const decoded = commentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', - 'Invalid value "1" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', ]); expect(message.schema).toEqual({}); }); diff --git a/x-pack/plugins/lists/common/schemas/types/comments.ts b/x-pack/plugins/lists/common/schemas/types/comment.ts similarity index 56% rename from x-pack/plugins/lists/common/schemas/types/comments.ts rename to x-pack/plugins/lists/common/schemas/types/comment.ts index 0ee3b05c8102f..6b0b0166b9ee1 100644 --- a/x-pack/plugins/lists/common/schemas/types/comments.ts +++ b/x-pack/plugins/lists/common/schemas/types/comment.ts @@ -3,26 +3,33 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ + +/* eslint-disable @typescript-eslint/camelcase */ + import * as t from 'io-ts'; -export const comments = t.intersection([ +import { NonEmptyString } from '../../siem_common_deps'; +import { created_at, created_by, id, updated_at, updated_by } from '../common/schemas'; + +export const comment = t.intersection([ t.exact( t.type({ - comment: t.string, - created_at: t.string, // TODO: Make this into an ISO Date string check, - created_by: t.string, + comment: NonEmptyString, + created_at, + created_by, + id, }) ), t.exact( t.partial({ - updated_at: t.string, - updated_by: t.string, + updated_at, + updated_by, }) ), ]); -export const commentsArray = t.array(comments); +export const commentsArray = t.array(comment); export type CommentsArray = t.TypeOf; -export type Comments = t.TypeOf; +export type Comment = t.TypeOf; export const commentsArrayOrUndefined = t.union([commentsArray, t.undefined]); export type CommentsArrayOrUndefined = t.TypeOf; diff --git a/x-pack/plugins/lists/common/schemas/types/create_comments.mock.ts b/x-pack/plugins/lists/common/schemas/types/create_comment.mock.ts similarity index 73% rename from x-pack/plugins/lists/common/schemas/types/create_comments.mock.ts rename to x-pack/plugins/lists/common/schemas/types/create_comment.mock.ts index 60a59432275ca..689d4ccdc2c2e 100644 --- a/x-pack/plugins/lists/common/schemas/types/create_comments.mock.ts +++ b/x-pack/plugins/lists/common/schemas/types/create_comment.mock.ts @@ -3,9 +3,9 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { CreateComments, CreateCommentsArray } from './create_comments'; +import { CreateComment, CreateCommentsArray } from './create_comment'; -export const getCreateCommentsMock = (): CreateComments => ({ +export const getCreateCommentsMock = (): CreateComment => ({ comment: 'some comments', }); diff --git a/x-pack/plugins/lists/common/schemas/types/create_comments.test.ts b/x-pack/plugins/lists/common/schemas/types/create_comment.test.ts similarity index 72% rename from x-pack/plugins/lists/common/schemas/types/create_comments.test.ts rename to x-pack/plugins/lists/common/schemas/types/create_comment.test.ts index d2680750e05e4..366bf84d48bbf 100644 --- a/x-pack/plugins/lists/common/schemas/types/create_comments.test.ts +++ b/x-pack/plugins/lists/common/schemas/types/create_comment.test.ts @@ -9,44 +9,44 @@ import { left } from 'fp-ts/lib/Either'; import { foldLeftRight, getPaths } from '../../siem_common_deps'; -import { getCreateCommentsArrayMock, getCreateCommentsMock } from './create_comments.mock'; +import { getCreateCommentsArrayMock, getCreateCommentsMock } from './create_comment.mock'; import { - CreateComments, + CreateComment, CreateCommentsArray, CreateCommentsArrayOrUndefined, - createComments, + createComment, createCommentsArray, createCommentsArrayOrUndefined, -} from './create_comments'; +} from './create_comment'; -describe('CreateComments', () => { - describe('createComments', () => { - test('it should validate a comments', () => { +describe('CreateComment', () => { + describe('createComment', () => { + test('it passes validation with a default comment', () => { const payload = getCreateCommentsMock(); - const decoded = createComments.decode(payload); + const decoded = createComment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([]); expect(message.schema).toEqual(payload); }); - test('it should not validate when undefined', () => { + test('it fails validation when undefined', () => { const payload = undefined; - const decoded = createComments.decode(payload); + const decoded = createComment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "undefined" supplied to "{| comment: string |}"', + 'Invalid value "undefined" supplied to "{| comment: NonEmptyString |}"', ]); expect(message.schema).toEqual({}); }); - test('it should not validate when "comment" is not a string', () => { - const payload: Omit & { comment: string[] } = { + test('it fails validation when "comment" is not a string', () => { + const payload: Omit & { comment: string[] } = { ...getCreateCommentsMock(), comment: ['some value'], }; - const decoded = createComments.decode(payload); + const decoded = createComment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ @@ -56,11 +56,11 @@ describe('CreateComments', () => { }); test('it should strip out extra keys', () => { - const payload: CreateComments & { + const payload: CreateComment & { extraKey?: string; } = getCreateCommentsMock(); payload.extraKey = 'some value'; - const decoded = createComments.decode(payload); + const decoded = createComment.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([]); @@ -69,7 +69,7 @@ describe('CreateComments', () => { }); describe('createCommentsArray', () => { - test('it should validate an array of comments', () => { + test('it passes validation an array of comments', () => { const payload = getCreateCommentsArrayMock(); const decoded = createCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -78,31 +78,31 @@ describe('CreateComments', () => { expect(message.schema).toEqual(payload); }); - test('it should not validate when undefined', () => { + test('it fails validation when undefined', () => { const payload = undefined; const decoded = createCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "undefined" supplied to "Array<{| comment: string |}>"', + 'Invalid value "undefined" supplied to "Array<{| comment: NonEmptyString |}>"', ]); expect(message.schema).toEqual({}); }); - test('it should not validate when array includes non comments types', () => { + test('it fails validation when array includes non comments types', () => { const payload = ([1] as unknown) as CreateCommentsArray; const decoded = createCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<{| comment: string |}>"', + 'Invalid value "1" supplied to "Array<{| comment: NonEmptyString |}>"', ]); expect(message.schema).toEqual({}); }); }); describe('createCommentsArrayOrUndefined', () => { - test('it should validate an array of comments', () => { + test('it passes validation an array of comments', () => { const payload = getCreateCommentsArrayMock(); const decoded = createCommentsArrayOrUndefined.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -111,7 +111,7 @@ describe('CreateComments', () => { expect(message.schema).toEqual(payload); }); - test('it should validate when undefined', () => { + test('it passes validation when undefined', () => { const payload = undefined; const decoded = createCommentsArrayOrUndefined.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -120,13 +120,13 @@ describe('CreateComments', () => { expect(message.schema).toEqual(payload); }); - test('it should not validate when array includes non comments types', () => { + test('it fails validation when array includes non comments types', () => { const payload = ([1] as unknown) as CreateCommentsArrayOrUndefined; const decoded = createCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<{| comment: string |}>"', + 'Invalid value "1" supplied to "Array<{| comment: NonEmptyString |}>"', ]); expect(message.schema).toEqual({}); }); diff --git a/x-pack/plugins/lists/common/schemas/types/create_comments.ts b/x-pack/plugins/lists/common/schemas/types/create_comment.ts similarity index 64% rename from x-pack/plugins/lists/common/schemas/types/create_comments.ts rename to x-pack/plugins/lists/common/schemas/types/create_comment.ts index c34419298ef93..fd33313430ce6 100644 --- a/x-pack/plugins/lists/common/schemas/types/create_comments.ts +++ b/x-pack/plugins/lists/common/schemas/types/create_comment.ts @@ -5,14 +5,17 @@ */ import * as t from 'io-ts'; -export const createComments = t.exact( +import { NonEmptyString } from '../../siem_common_deps'; + +export const createComment = t.exact( t.type({ - comment: t.string, + comment: NonEmptyString, }) ); -export const createCommentsArray = t.array(createComments); +export type CreateComment = t.TypeOf; +export const createCommentsArray = t.array(createComment); export type CreateCommentsArray = t.TypeOf; -export type CreateComments = t.TypeOf; +export type CreateComments = t.TypeOf; export const createCommentsArrayOrUndefined = t.union([createCommentsArray, t.undefined]); export type CreateCommentsArrayOrUndefined = t.TypeOf; diff --git a/x-pack/plugins/lists/common/schemas/types/default_comments_array.test.ts b/x-pack/plugins/lists/common/schemas/types/default_comments_array.test.ts index 3a4241aaec82d..541b8ab1c799c 100644 --- a/x-pack/plugins/lists/common/schemas/types/default_comments_array.test.ts +++ b/x-pack/plugins/lists/common/schemas/types/default_comments_array.test.ts @@ -10,11 +10,11 @@ import { left } from 'fp-ts/lib/Either'; import { foldLeftRight, getPaths } from '../../siem_common_deps'; import { DefaultCommentsArray } from './default_comments_array'; -import { CommentsArray } from './comments'; -import { getCommentsArrayMock } from './comments.mock'; +import { CommentsArray } from './comment'; +import { getCommentsArrayMock } from './comment.mock'; describe('default_comments_array', () => { - test('it should validate an empty array', () => { + test('it should pass validation when supplied an empty array', () => { const payload: CommentsArray = []; const decoded = DefaultCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -23,7 +23,7 @@ describe('default_comments_array', () => { expect(message.schema).toEqual(payload); }); - test('it should validate an array of comments', () => { + test('it should pass validation when supplied an array of comments', () => { const payload: CommentsArray = getCommentsArrayMock(); const decoded = DefaultCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -32,27 +32,26 @@ describe('default_comments_array', () => { expect(message.schema).toEqual(payload); }); - test('it should NOT validate an array of numbers', () => { + test('it should fail validation when supplied an array of numbers', () => { const payload = [1]; const decoded = DefaultCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); - // TODO: Known weird error formatting that is on our list to address expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', - 'Invalid value "1" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', ]); expect(message.schema).toEqual({}); }); - test('it should NOT validate an array of strings', () => { + test('it should fail validation when supplied an array of strings', () => { const payload = ['some string']; const decoded = DefaultCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "some string" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', - 'Invalid value "some string" supplied to "Array<({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', + 'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"', ]); expect(message.schema).toEqual({}); }); diff --git a/x-pack/plugins/lists/common/schemas/types/default_comments_array.ts b/x-pack/plugins/lists/common/schemas/types/default_comments_array.ts index 342cf8b0d7091..0d7e28e69cf71 100644 --- a/x-pack/plugins/lists/common/schemas/types/default_comments_array.ts +++ b/x-pack/plugins/lists/common/schemas/types/default_comments_array.ts @@ -7,7 +7,7 @@ import * as t from 'io-ts'; import { Either } from 'fp-ts/lib/Either'; -import { CommentsArray, comments } from './comments'; +import { CommentsArray, comment } from './comment'; /** * Types the DefaultCommentsArray as: @@ -15,8 +15,8 @@ import { CommentsArray, comments } from './comments'; */ export const DefaultCommentsArray = new t.Type( 'DefaultCommentsArray', - t.array(comments).is, + t.array(comment).is, (input): Either => - input == null ? t.success([]) : t.array(comments).decode(input), + input == null ? t.success([]) : t.array(comment).decode(input), t.identity ); diff --git a/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.test.ts b/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.test.ts index f5ef7d0ad96bd..eb960b5411904 100644 --- a/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.test.ts +++ b/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.test.ts @@ -10,11 +10,12 @@ import { left } from 'fp-ts/lib/Either'; import { foldLeftRight, getPaths } from '../../siem_common_deps'; import { DefaultCreateCommentsArray } from './default_create_comments_array'; -import { CreateCommentsArray } from './create_comments'; -import { getCreateCommentsArrayMock } from './create_comments.mock'; +import { CreateCommentsArray } from './create_comment'; +import { getCreateCommentsArrayMock } from './create_comment.mock'; +import { getCommentsArrayMock } from './comment.mock'; describe('default_create_comments_array', () => { - test('it should validate an empty array', () => { + test('it should pass validation when an empty array', () => { const payload: CreateCommentsArray = []; const decoded = DefaultCreateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -23,7 +24,7 @@ describe('default_create_comments_array', () => { expect(message.schema).toEqual(payload); }); - test('it should validate an array of comments', () => { + test('it should pass validation when an array of comments', () => { const payload: CreateCommentsArray = getCreateCommentsArrayMock(); const decoded = DefaultCreateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -32,25 +33,38 @@ describe('default_create_comments_array', () => { expect(message.schema).toEqual(payload); }); - test('it should NOT validate an array of numbers', () => { + test('it should strip out "created_at" and "created_by" if they are passed in', () => { + const payload = getCommentsArrayMock(); + const decoded = DefaultCreateCommentsArray.decode(payload); + const message = pipe(decoded, foldLeftRight); + + // TODO: Known weird error formatting that is on our list to address + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual([ + { comment: 'some old comment' }, + { comment: 'some old comment' }, + ]); + }); + + test('it should not pass validation when an array of numbers', () => { const payload = [1]; const decoded = DefaultCreateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); // TODO: Known weird error formatting that is on our list to address expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<{| comment: string |}>"', + 'Invalid value "1" supplied to "Array<{| comment: NonEmptyString |}>"', ]); expect(message.schema).toEqual({}); }); - test('it should NOT validate an array of strings', () => { + test('it should not pass validation when an array of strings', () => { const payload = ['some string']; const decoded = DefaultCreateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "some string" supplied to "Array<{| comment: string |}>"', + 'Invalid value "some string" supplied to "Array<{| comment: NonEmptyString |}>"', ]); expect(message.schema).toEqual({}); }); diff --git a/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.ts b/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.ts index 7fd79782836e3..4df888ba728fb 100644 --- a/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.ts +++ b/x-pack/plugins/lists/common/schemas/types/default_create_comments_array.ts @@ -7,7 +7,7 @@ import * as t from 'io-ts'; import { Either } from 'fp-ts/lib/Either'; -import { CreateCommentsArray, createComments } from './create_comments'; +import { CreateCommentsArray, createComment } from './create_comment'; /** * Types the DefaultCreateComments as: @@ -19,8 +19,8 @@ export const DefaultCreateCommentsArray = new t.Type< unknown >( 'DefaultCreateComments', - t.array(createComments).is, + t.array(createComment).is, (input): Either => - input == null ? t.success([]) : t.array(createComments).decode(input), + input == null ? t.success([]) : t.array(createComment).decode(input), t.identity ); diff --git a/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.test.ts b/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.test.ts index b023e73cb9328..612148dc4ccab 100644 --- a/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.test.ts +++ b/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.test.ts @@ -10,11 +10,11 @@ import { left } from 'fp-ts/lib/Either'; import { foldLeftRight, getPaths } from '../../siem_common_deps'; import { DefaultUpdateCommentsArray } from './default_update_comments_array'; -import { UpdateCommentsArray } from './update_comments'; -import { getUpdateCommentsArrayMock } from './update_comments.mock'; +import { UpdateCommentsArray } from './update_comment'; +import { getUpdateCommentsArrayMock } from './update_comment.mock'; describe('default_update_comments_array', () => { - test('it should validate an empty array', () => { + test('it should pass validation when supplied an empty array', () => { const payload: UpdateCommentsArray = []; const decoded = DefaultUpdateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -23,7 +23,7 @@ describe('default_update_comments_array', () => { expect(message.schema).toEqual(payload); }); - test('it should validate an array of comments', () => { + test('it should pass validation when supplied an array of comments', () => { const payload: UpdateCommentsArray = getUpdateCommentsArrayMock(); const decoded = DefaultUpdateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); @@ -32,29 +32,26 @@ describe('default_update_comments_array', () => { expect(message.schema).toEqual(payload); }); - test('it should NOT validate an array of numbers', () => { + test('it should fail validation when supplied an array of numbers', () => { const payload = [1]; const decoded = DefaultUpdateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); - // TODO: Known weird error formatting that is on our list to address expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', ]); expect(message.schema).toEqual({}); }); - test('it should NOT validate an array of strings', () => { + test('it should fail validation when supplied an array of strings', () => { const payload = ['some string']; const decoded = DefaultUpdateCommentsArray.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "some string" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "some string" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "some string" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', + 'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + 'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', ]); expect(message.schema).toEqual({}); }); diff --git a/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.ts b/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.ts index 854b7cf7ada7e..35338dae64387 100644 --- a/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.ts +++ b/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.ts @@ -7,7 +7,7 @@ import * as t from 'io-ts'; import { Either } from 'fp-ts/lib/Either'; -import { UpdateCommentsArray, updateCommentsArray } from './update_comments'; +import { UpdateCommentsArray, updateCommentsArray } from './update_comment'; /** * Types the DefaultCommentsUpdate as: diff --git a/x-pack/plugins/lists/common/schemas/types/index.ts b/x-pack/plugins/lists/common/schemas/types/index.ts index 463f7cfe51ce3..6b7e9fd17a1af 100644 --- a/x-pack/plugins/lists/common/schemas/types/index.ts +++ b/x-pack/plugins/lists/common/schemas/types/index.ts @@ -3,9 +3,9 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -export * from './comments'; -export * from './create_comments'; -export * from './update_comments'; +export * from './comment'; +export * from './create_comment'; +export * from './update_comment'; export * from './default_comments_array'; export * from './default_create_comments_array'; export * from './default_update_comments_array'; diff --git a/x-pack/plugins/lists/common/schemas/types/update_comments.mock.ts b/x-pack/plugins/lists/common/schemas/types/update_comment.mock.ts similarity index 54% rename from x-pack/plugins/lists/common/schemas/types/update_comments.mock.ts rename to x-pack/plugins/lists/common/schemas/types/update_comment.mock.ts index 3e963c2607dc5..9b85a24abe40b 100644 --- a/x-pack/plugins/lists/common/schemas/types/update_comments.mock.ts +++ b/x-pack/plugins/lists/common/schemas/types/update_comment.mock.ts @@ -4,11 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getCommentsMock } from './comments.mock'; -import { getCreateCommentsMock } from './create_comments.mock'; -import { UpdateCommentsArray } from './update_comments'; +import { ID } from '../../constants.mock'; + +import { UpdateComment, UpdateCommentsArray } from './update_comment'; + +export const getUpdateCommentMock = (): UpdateComment => ({ + comment: 'some comment', + id: ID, +}); export const getUpdateCommentsArrayMock = (): UpdateCommentsArray => [ - getCommentsMock(), - getCreateCommentsMock(), + getUpdateCommentMock(), + getUpdateCommentMock(), ]; diff --git a/x-pack/plugins/lists/common/schemas/types/update_comment.test.ts b/x-pack/plugins/lists/common/schemas/types/update_comment.test.ts new file mode 100644 index 0000000000000..ac7716af40966 --- /dev/null +++ b/x-pack/plugins/lists/common/schemas/types/update_comment.test.ts @@ -0,0 +1,150 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { pipe } from 'fp-ts/lib/pipeable'; +import { left } from 'fp-ts/lib/Either'; + +import { foldLeftRight, getPaths } from '../../siem_common_deps'; + +import { getUpdateCommentMock, getUpdateCommentsArrayMock } from './update_comment.mock'; +import { + UpdateComment, + UpdateCommentsArray, + UpdateCommentsArrayOrUndefined, + updateComment, + updateCommentsArray, + updateCommentsArrayOrUndefined, +} from './update_comment'; + +describe('CommentsUpdate', () => { + describe('updateComment', () => { + test('it should pass validation when supplied typical comment update', () => { + const payload = getUpdateCommentMock(); + const decoded = updateComment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('it should fail validation when supplied an undefined for "comment"', () => { + const payload = getUpdateCommentMock(); + delete payload.comment; + const decoded = updateComment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "comment"', + ]); + expect(message.schema).toEqual({}); + }); + + test('it should fail validation when supplied an empty string for "comment"', () => { + const payload = { ...getUpdateCommentMock(), comment: '' }; + const decoded = updateComment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual(['Invalid value "" supplied to "comment"']); + expect(message.schema).toEqual({}); + }); + + test('it should pass validation when supplied an undefined for "id"', () => { + const payload = getUpdateCommentMock(); + delete payload.id; + const decoded = updateComment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('it should fail validation when supplied an empty string for "id"', () => { + const payload = { ...getUpdateCommentMock(), id: '' }; + const decoded = updateComment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual(['Invalid value "" supplied to "id"']); + expect(message.schema).toEqual({}); + }); + + test('it should strip out extra key passed in', () => { + const payload: UpdateComment & { + extraKey?: string; + } = { ...getUpdateCommentMock(), extraKey: 'some new value' }; + const decoded = updateComment.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(getUpdateCommentMock()); + }); + }); + + describe('updateCommentsArray', () => { + test('it should pass validation when supplied an array of comments', () => { + const payload = getUpdateCommentsArrayMock(); + const decoded = updateCommentsArray.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('it should fail validation when undefined', () => { + const payload = undefined; + const decoded = updateCommentsArray.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + ]); + expect(message.schema).toEqual({}); + }); + + test('it should fail validation when array includes non comments types', () => { + const payload = ([1] as unknown) as UpdateCommentsArray; + const decoded = updateCommentsArray.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + ]); + expect(message.schema).toEqual({}); + }); + }); + + describe('updateCommentsArrayOrUndefined', () => { + test('it should pass validation when supplied an array of comments', () => { + const payload = getUpdateCommentsArrayMock(); + const decoded = updateCommentsArrayOrUndefined.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('it should pass validation when supplied when undefined', () => { + const payload = undefined; + const decoded = updateCommentsArrayOrUndefined.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('it should fail validation when array includes non comments types', () => { + const payload = ([1] as unknown) as UpdateCommentsArrayOrUndefined; + const decoded = updateCommentsArray.decode(payload); + const message = pipe(decoded, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + 'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"', + ]); + expect(message.schema).toEqual({}); + }); + }); +}); diff --git a/x-pack/plugins/lists/common/schemas/types/update_comments.ts b/x-pack/plugins/lists/common/schemas/types/update_comment.ts similarity index 58% rename from x-pack/plugins/lists/common/schemas/types/update_comments.ts rename to x-pack/plugins/lists/common/schemas/types/update_comment.ts index 4a21bfa363d45..b95812cb35bf9 100644 --- a/x-pack/plugins/lists/common/schemas/types/update_comments.ts +++ b/x-pack/plugins/lists/common/schemas/types/update_comment.ts @@ -5,10 +5,24 @@ */ import * as t from 'io-ts'; -import { comments } from './comments'; -import { createComments } from './create_comments'; +import { NonEmptyString } from '../../siem_common_deps'; +import { id } from '../common/schemas'; -export const updateCommentsArray = t.array(t.union([comments, createComments])); +export const updateComment = t.intersection([ + t.exact( + t.type({ + comment: NonEmptyString, + }) + ), + t.exact( + t.partial({ + id, + }) + ), +]); + +export type UpdateComment = t.TypeOf; +export const updateCommentsArray = t.array(updateComment); export type UpdateCommentsArray = t.TypeOf; export const updateCommentsArrayOrUndefined = t.union([updateCommentsArray, t.undefined]); export type UpdateCommentsArrayOrUndefined = t.TypeOf; diff --git a/x-pack/plugins/lists/common/schemas/types/update_comments.test.ts b/x-pack/plugins/lists/common/schemas/types/update_comments.test.ts deleted file mode 100644 index 7668504b031b5..0000000000000 --- a/x-pack/plugins/lists/common/schemas/types/update_comments.test.ts +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { pipe } from 'fp-ts/lib/pipeable'; -import { left } from 'fp-ts/lib/Either'; - -import { foldLeftRight, getPaths } from '../../siem_common_deps'; - -import { getUpdateCommentsArrayMock } from './update_comments.mock'; -import { - UpdateCommentsArray, - UpdateCommentsArrayOrUndefined, - updateCommentsArray, - updateCommentsArrayOrUndefined, -} from './update_comments'; -import { getCommentsMock } from './comments.mock'; -import { getCreateCommentsMock } from './create_comments.mock'; - -describe('CommentsUpdate', () => { - describe('updateCommentsArray', () => { - test('it should validate an array of comments', () => { - const payload = getUpdateCommentsArrayMock(); - const decoded = updateCommentsArray.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([]); - expect(message.schema).toEqual(payload); - }); - - test('it should validate an array of existing comments', () => { - const payload = [getCommentsMock()]; - const decoded = updateCommentsArray.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([]); - expect(message.schema).toEqual(payload); - }); - - test('it should validate an array of new comments', () => { - const payload = [getCreateCommentsMock()]; - const decoded = updateCommentsArray.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([]); - expect(message.schema).toEqual(payload); - }); - - test('it should not validate when undefined', () => { - const payload = undefined; - const decoded = updateCommentsArray.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "undefined" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - ]); - expect(message.schema).toEqual({}); - }); - - test('it should not validate when array includes non comments types', () => { - const payload = ([1] as unknown) as UpdateCommentsArray; - const decoded = updateCommentsArray.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - ]); - expect(message.schema).toEqual({}); - }); - }); - - describe('updateCommentsArrayOrUndefined', () => { - test('it should validate an array of comments', () => { - const payload = getUpdateCommentsArrayMock(); - const decoded = updateCommentsArrayOrUndefined.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([]); - expect(message.schema).toEqual(payload); - }); - - test('it should validate when undefined', () => { - const payload = undefined; - const decoded = updateCommentsArrayOrUndefined.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([]); - expect(message.schema).toEqual(payload); - }); - - test('it should not validate when array includes non comments types', () => { - const payload = ([1] as unknown) as UpdateCommentsArrayOrUndefined; - const decoded = updateCommentsArray.decode(payload); - const message = pipe(decoded, foldLeftRight); - - expect(getPaths(left(message.errors))).toEqual([ - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - 'Invalid value "1" supplied to "Array<(({| comment: string, created_at: string, created_by: string |} & Partial<{| updated_at: string, updated_by: string |}>) | {| comment: string |})>"', - ]); - expect(message.schema).toEqual({}); - }); - }); -}); diff --git a/x-pack/plugins/lists/common/shared_exports.ts b/x-pack/plugins/lists/common/shared_exports.ts index dc0a9aa5926ef..1f6c65919b063 100644 --- a/x-pack/plugins/lists/common/shared_exports.ts +++ b/x-pack/plugins/lists/common/shared_exports.ts @@ -8,8 +8,8 @@ export { ListSchema, CommentsArray, CreateCommentsArray, - Comments, - CreateComments, + Comment, + CreateComment, ExceptionListSchema, ExceptionListItemSchema, CreateExceptionListSchema, @@ -28,6 +28,7 @@ export { OperatorType, OperatorTypeEnum, ExceptionListTypeEnum, + comment, exceptionListItemSchema, exceptionListType, createExceptionListItemSchema, diff --git a/x-pack/plugins/lists/server/routes/update_exception_list_item_route.ts b/x-pack/plugins/lists/server/routes/update_exception_list_item_route.ts index 293435b3f6202..f5e0e7ae75700 100644 --- a/x-pack/plugins/lists/server/routes/update_exception_list_item_route.ts +++ b/x-pack/plugins/lists/server/routes/update_exception_list_item_route.ts @@ -14,6 +14,7 @@ import { exceptionListItemSchema, updateExceptionListItemSchema, } from '../../common/schemas'; +import { updateExceptionListItemValidate } from '../../common/schemas/request/update_exception_list_item_validation'; import { getExceptionListClient } from '.'; @@ -33,6 +34,11 @@ export const updateExceptionListItemRoute = (router: IRouter): void => { }, async (context, request, response) => { const siemResponse = buildSiemResponse(response); + const validationErrors = updateExceptionListItemValidate(request.body); + if (validationErrors.length) { + return siemResponse.error({ body: validationErrors, statusCode: 400 }); + } + try { const { description, diff --git a/x-pack/plugins/lists/server/saved_objects/exception_list.ts b/x-pack/plugins/lists/server/saved_objects/exception_list.ts index 3bde3545837cf..f9e408833e069 100644 --- a/x-pack/plugins/lists/server/saved_objects/exception_list.ts +++ b/x-pack/plugins/lists/server/saved_objects/exception_list.ts @@ -83,6 +83,9 @@ export const exceptionListItemMapping: SavedObjectsType['mappings'] = { created_by: { type: 'keyword', }, + id: { + type: 'keyword', + }, updated_at: { type: 'keyword', }, diff --git a/x-pack/plugins/lists/server/scripts/exception_lists/updates/simple_update_item.json b/x-pack/plugins/lists/server/scripts/exception_lists/updates/simple_update_item.json index da345fb930c04..81db909277595 100644 --- a/x-pack/plugins/lists/server/scripts/exception_lists/updates/simple_update_item.json +++ b/x-pack/plugins/lists/server/scripts/exception_lists/updates/simple_update_item.json @@ -1,17 +1,18 @@ { - "item_id": "simple_list_item", - "_tags": ["endpoint", "process", "malware", "os:windows"], - "tags": ["user added string for a tag", "malware"], - "type": "simple", - "description": "This is a sample change here this list", - "name": "Sample Endpoint Exception List update change", - "comments": [{ "comment": "this is a newly added comment" }], + "_tags": ["detection"], + "comments": [], + "description": "Test comments - exception list item", "entries": [ { - "field": "event.category", - "operator": "included", - "type": "match_any", - "value": ["process", "malware"] + "field": "host.name", + "type": "match", + "value": "rock01", + "operator": "included" } - ] + ], + "item_id": "993f43f7-325d-4df3-9338-964e77c37053", + "name": "Test comments - exception list item", + "namespace_type": "single", + "tags": [], + "type": "simple" } diff --git a/x-pack/plugins/lists/server/services/exception_lists/create_exception_list_item.ts b/x-pack/plugins/lists/server/services/exception_lists/create_exception_list_item.ts index a90ec61aef4af..47c21735b45f4 100644 --- a/x-pack/plugins/lists/server/services/exception_lists/create_exception_list_item.ts +++ b/x-pack/plugins/lists/server/services/exception_lists/create_exception_list_item.ts @@ -64,7 +64,10 @@ export const createExceptionListItem = async ({ }: CreateExceptionListItemOptions): Promise => { const savedObjectType = getSavedObjectType({ namespaceType }); const dateNow = new Date().toISOString(); - const transformedComments = transformCreateCommentsToComments({ comments, user }); + const transformedComments = transformCreateCommentsToComments({ + incomingComments: comments, + user, + }); const savedObject = await savedObjectsClient.create(savedObjectType, { _tags, comments: transformedComments, diff --git a/x-pack/plugins/lists/server/services/exception_lists/utils.test.ts b/x-pack/plugins/lists/server/services/exception_lists/utils.test.ts index 6f0c5195f2025..e3d96a9c3f6d0 100644 --- a/x-pack/plugins/lists/server/services/exception_lists/utils.test.ts +++ b/x-pack/plugins/lists/server/services/exception_lists/utils.test.ts @@ -5,15 +5,11 @@ */ import sinon from 'sinon'; import moment from 'moment'; +import uuid from 'uuid'; -import { USER } from '../../../common/constants.mock'; +import { transformCreateCommentsToComments, transformUpdateCommentsToComments } from './utils'; -import { - isCommentEqual, - transformCreateCommentsToComments, - transformUpdateComments, - transformUpdateCommentsToComments, -} from './utils'; +jest.mock('uuid/v4'); describe('utils', () => { const oldDate = '2020-03-17T20:34:51.337Z'; @@ -22,59 +18,43 @@ describe('utils', () => { let clock: sinon.SinonFakeTimers; beforeEach(() => { + ((uuid.v4 as unknown) as jest.Mock) + .mockImplementationOnce(() => '123') + .mockImplementationOnce(() => '456'); + clock = sinon.useFakeTimers(unix); }); afterEach(() => { clock.restore(); + jest.clearAllMocks(); + jest.restoreAllMocks(); + jest.resetAllMocks(); }); describe('#transformUpdateCommentsToComments', () => { - test('it returns empty array if "comments" is undefined and no comments exist', () => { + test('it formats new comments', () => { const comments = transformUpdateCommentsToComments({ - comments: undefined, + comments: [{ comment: 'Im a new comment' }], existingComments: [], user: 'lily', }); - expect(comments).toEqual([]); - }); - - test('it formats newly added comments', () => { - const comments = transformUpdateCommentsToComments({ - comments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'bane' }, - { comment: 'Im a new comment' }, - ], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'bane' }, - ], - user: 'lily', - }); - expect(comments).toEqual([ - { - comment: 'Im an old comment', - created_at: oldDate, - created_by: 'bane', - }, { comment: 'Im a new comment', created_at: dateNow, created_by: 'lily', + id: '123', }, ]); }); - test('it formats multiple newly added comments', () => { + test('it formats new comments and preserves existing comments', () => { const comments = transformUpdateCommentsToComments({ - comments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - { comment: 'Im a new comment' }, - { comment: 'Im another new comment' }, - ], + comments: [{ comment: 'Im an old comment', id: '1' }, { comment: 'Im a new comment' }], existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, + { comment: 'Im an old comment', created_at: oldDate, created_by: 'bane', id: '1' }, ], user: 'lily', }); @@ -83,26 +63,23 @@ describe('utils', () => { { comment: 'Im an old comment', created_at: oldDate, - created_by: 'lily', + created_by: 'bane', + id: '1', }, { comment: 'Im a new comment', created_at: dateNow, created_by: 'lily', - }, - { - comment: 'Im another new comment', - created_at: dateNow, - created_by: 'lily', + id: '123', }, ]); }); - test('it should not throw if comments match existing comments', () => { + test('it returns existing comments if empty array passed for "comments"', () => { const comments = transformUpdateCommentsToComments({ - comments: [{ comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }], + comments: [], existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, + { comment: 'Im an old comment', created_at: oldDate, created_by: 'bane', id: '1' }, ], user: 'lily', }); @@ -111,170 +88,42 @@ describe('utils', () => { { comment: 'Im an old comment', created_at: oldDate, - created_by: 'lily', + created_by: 'bane', + id: '1', }, ]); }); - test('it does not throw if user tries to update one of their own existing comments', () => { + test('it acts as append only, only modifying new comments', () => { const comments = transformUpdateCommentsToComments({ - comments: [ - { - comment: 'Im an old comment that is trying to be updated', - created_at: oldDate, - created_by: 'lily', - }, - ], + comments: [{ comment: 'Im a new comment' }], existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, + { comment: 'Im an old comment', created_at: oldDate, created_by: 'bane', id: '1' }, ], user: 'lily', }); expect(comments).toEqual([ { - comment: 'Im an old comment that is trying to be updated', + comment: 'Im an old comment', created_at: oldDate, + created_by: 'bane', + id: '1', + }, + { + comment: 'Im a new comment', + created_at: dateNow, created_by: 'lily', - updated_at: dateNow, - updated_by: 'lily', + id: '123', }, ]); }); - - test('it throws an error if user tries to update their comment, without passing in the "created_at" and "created_by" properties', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [ - { - comment: 'Im an old comment that is trying to be updated', - }, - ], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot( - `"When trying to update a comment, \\"created_at\\" and \\"created_by\\" must be present"` - ); - }); - - test('it throws an error if user tries to delete comments', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot( - `"Comments cannot be deleted, only new comments may be added"` - ); - }); - - test('it throws if user tries to update existing comment timestamp', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [{ comment: 'Im an old comment', created_at: dateNow, created_by: 'lily' }], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - user: 'bane', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Not authorized to edit others comments"`); - }); - - test('it throws if user tries to update existing comment author', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [{ comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'me!' }, - ], - user: 'bane', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Not authorized to edit others comments"`); - }); - - test('it throws if user tries to update an existing comment that is not their own', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [ - { - comment: 'Im an old comment that is trying to be updated', - created_at: oldDate, - created_by: 'lily', - }, - ], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - user: 'bane', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Not authorized to edit others comments"`); - }); - - test('it throws if user tries to update order of comments', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [ - { comment: 'Im a new comment' }, - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot( - `"When trying to update a comment, \\"created_at\\" and \\"created_by\\" must be present"` - ); - }); - - test('it throws an error if user tries to add comment formatted as existing comment when none yet exist', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - { comment: 'Im a new comment' }, - ], - existingComments: [], - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Only new comments may be added"`); - }); - - test('it throws if empty comment exists', () => { - expect(() => - transformUpdateCommentsToComments({ - comments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - { comment: ' ' }, - ], - existingComments: [ - { comment: 'Im an old comment', created_at: oldDate, created_by: 'lily' }, - ], - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Empty comments not allowed"`); - }); }); describe('#transformCreateCommentsToComments', () => { - test('it returns "undefined" if "comments" is "undefined"', () => { - const comments = transformCreateCommentsToComments({ - comments: undefined, - user: 'lily', - }); - - expect(comments).toBeUndefined(); - }); - test('it formats newly added comments', () => { const comments = transformCreateCommentsToComments({ - comments: [{ comment: 'Im a new comment' }, { comment: 'Im another new comment' }], + incomingComments: [{ comment: 'Im a new comment' }, { comment: 'Im another new comment' }], user: 'lily', }); @@ -283,178 +132,15 @@ describe('utils', () => { comment: 'Im a new comment', created_at: dateNow, created_by: 'lily', + id: '123', }, { comment: 'Im another new comment', created_at: dateNow, created_by: 'lily', + id: '456', }, ]); }); - - test('it throws an error if user tries to add an empty comment', () => { - expect(() => - transformCreateCommentsToComments({ - comments: [{ comment: ' ' }], - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Empty comments not allowed"`); - }); - }); - - describe('#transformUpdateComments', () => { - test('it updates comment and adds "updated_at" and "updated_by" if content differs', () => { - const comments = transformUpdateComments({ - comment: { - comment: 'Im an old comment that is trying to be updated', - created_at: oldDate, - created_by: 'lily', - }, - existingComment: { - comment: 'Im an old comment', - created_at: oldDate, - created_by: 'lily', - }, - user: 'lily', - }); - - expect(comments).toEqual({ - comment: 'Im an old comment that is trying to be updated', - created_at: oldDate, - created_by: 'lily', - updated_at: dateNow, - updated_by: 'lily', - }); - }); - - test('it does not update comment and add "updated_at" and "updated_by" if content is the same', () => { - const comments = transformUpdateComments({ - comment: { - comment: 'Im an old comment ', - created_at: oldDate, - created_by: 'lily', - }, - existingComment: { - comment: 'Im an old comment', - created_at: oldDate, - created_by: 'lily', - }, - user: 'lily', - }); - - expect(comments).toEqual({ - comment: 'Im an old comment', - created_at: oldDate, - created_by: 'lily', - }); - }); - - test('it throws if user tries to update an existing comment that is not their own', () => { - expect(() => - transformUpdateComments({ - comment: { - comment: 'Im an old comment that is trying to be updated', - created_at: oldDate, - created_by: 'lily', - }, - existingComment: { - comment: 'Im an old comment', - created_at: oldDate, - created_by: 'lily', - }, - user: 'bane', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Not authorized to edit others comments"`); - }); - - test('it throws if user tries to update an existing comments timestamp', () => { - expect(() => - transformUpdateComments({ - comment: { - comment: 'Im an old comment', - created_at: dateNow, - created_by: 'lily', - }, - existingComment: { - comment: 'Im an old comment', - created_at: oldDate, - created_by: 'lily', - }, - user: 'lily', - }) - ).toThrowErrorMatchingInlineSnapshot(`"Unable to update comment"`); - }); - }); - - describe('#isCommentEqual', () => { - test('it returns false if "comment" values differ', () => { - const result = isCommentEqual( - { - comment: 'some old comment', - created_at: oldDate, - created_by: USER, - }, - { - comment: 'some older comment', - created_at: oldDate, - created_by: USER, - } - ); - - expect(result).toBeFalsy(); - }); - - test('it returns false if "created_at" values differ', () => { - const result = isCommentEqual( - { - comment: 'some old comment', - created_at: oldDate, - created_by: USER, - }, - { - comment: 'some old comment', - created_at: dateNow, - created_by: USER, - } - ); - - expect(result).toBeFalsy(); - }); - - test('it returns false if "created_by" values differ', () => { - const result = isCommentEqual( - { - comment: 'some old comment', - created_at: oldDate, - created_by: USER, - }, - { - comment: 'some old comment', - created_at: oldDate, - created_by: 'lily', - } - ); - - expect(result).toBeFalsy(); - }); - - test('it returns true if comment values are equivalent', () => { - const result = isCommentEqual( - { - comment: 'some old comment', - created_at: oldDate, - created_by: USER, - }, - { - created_at: oldDate, - created_by: USER, - // Disabling to assure that order doesn't matter - // eslint-disable-next-line sort-keys - comment: 'some old comment', - } - ); - - expect(result).toBeTruthy(); - }); }); }); diff --git a/x-pack/plugins/lists/server/services/exception_lists/utils.ts b/x-pack/plugins/lists/server/services/exception_lists/utils.ts index b168fae741822..836f642899086 100644 --- a/x-pack/plugins/lists/server/services/exception_lists/utils.ts +++ b/x-pack/plugins/lists/server/services/exception_lists/utils.ts @@ -3,17 +3,14 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ - +import uuid from 'uuid'; import { SavedObject, SavedObjectsFindResponse, SavedObjectsUpdateResponse } from 'kibana/server'; import { NamespaceTypeArray } from '../../../common/schemas/types/default_namespace_array'; -import { ErrorWithStatusCode } from '../../error_with_status_code'; import { - Comments, CommentsArray, - CommentsArrayOrUndefined, - CreateComments, - CreateCommentsArrayOrUndefined, + CreateComment, + CreateCommentsArray, ExceptionListItemSchema, ExceptionListSchema, ExceptionListSoSchema, @@ -21,7 +18,6 @@ import { FoundExceptionListSchema, NamespaceType, UpdateCommentsArrayOrUndefined, - comments as commentsSchema, exceptionListItemType, exceptionListType, } from '../../../common/schemas'; @@ -296,17 +292,6 @@ export const transformSavedObjectsToFoundExceptionList = ({ }; }; -/* - * Determines whether two comments are equal, this is a very - * naive implementation, not meant to be used for deep equality of complex objects - */ -export const isCommentEqual = (commentA: Comments, commentB: Comments): boolean => { - const a = Object.values(commentA).sort().join(); - const b = Object.values(commentB).sort().join(); - - return a === b; -}; - export const transformUpdateCommentsToComments = ({ comments, existingComments, @@ -316,90 +301,28 @@ export const transformUpdateCommentsToComments = ({ existingComments: CommentsArray; user: string; }): CommentsArray => { - const newComments = comments ?? []; + const incomingComments = comments ?? []; + const newComments = incomingComments.filter((comment) => comment.id == null); + const newCommentsFormatted = transformCreateCommentsToComments({ + incomingComments: newComments, + user, + }); - if (newComments.length < existingComments.length) { - throw new ErrorWithStatusCode( - 'Comments cannot be deleted, only new comments may be added', - 403 - ); - } else { - return newComments.flatMap((c, index) => { - const existingComment = existingComments[index]; - - if (commentsSchema.is(existingComment) && !commentsSchema.is(c)) { - throw new ErrorWithStatusCode( - 'When trying to update a comment, "created_at" and "created_by" must be present', - 403 - ); - } else if (existingComment == null && commentsSchema.is(c)) { - throw new ErrorWithStatusCode('Only new comments may be added', 403); - } else if ( - commentsSchema.is(c) && - existingComment != null && - isCommentEqual(c, existingComment) - ) { - return existingComment; - } else if (commentsSchema.is(c) && existingComment != null) { - return transformUpdateComments({ comment: c, existingComment, user }); - } else { - return transformCreateCommentsToComments({ comments: [c], user }) ?? []; - } - }); - } -}; - -export const transformUpdateComments = ({ - comment, - existingComment, - user, -}: { - comment: Comments; - existingComment: Comments; - user: string; -}): Comments => { - if (comment.created_by !== user) { - // existing comment is being edited, can only be edited by author - throw new ErrorWithStatusCode('Not authorized to edit others comments', 401); - } else if (existingComment.created_at !== comment.created_at) { - throw new ErrorWithStatusCode('Unable to update comment', 403); - } else if (comment.comment.trim().length === 0) { - throw new ErrorWithStatusCode('Empty comments not allowed', 403); - } else if (comment.comment.trim() !== existingComment.comment) { - const dateNow = new Date().toISOString(); - - return { - ...existingComment, - comment: comment.comment, - updated_at: dateNow, - updated_by: user, - }; - } else { - return existingComment; - } + return [...existingComments, ...newCommentsFormatted]; }; export const transformCreateCommentsToComments = ({ - comments, + incomingComments, user, }: { - comments: CreateCommentsArrayOrUndefined; + incomingComments: CreateCommentsArray; user: string; -}): CommentsArrayOrUndefined => { +}): CommentsArray => { const dateNow = new Date().toISOString(); - if (comments != null) { - return comments.map((c: CreateComments) => { - if (c.comment.trim().length === 0) { - throw new ErrorWithStatusCode('Empty comments not allowed', 403); - } else { - return { - comment: c.comment, - created_at: dateNow, - created_by: user, - }; - } - }); - } else { - return comments; - } + return incomingComments.map((comment: CreateComment) => ({ + comment: comment.comment, + created_at: dateNow, + created_by: user, + id: uuid.v4(), + })); }; diff --git a/x-pack/plugins/security_solution/common/shared_imports.ts b/x-pack/plugins/security_solution/common/shared_imports.ts index 7fb94cea7b612..e28d1969b3976 100644 --- a/x-pack/plugins/security_solution/common/shared_imports.ts +++ b/x-pack/plugins/security_solution/common/shared_imports.ts @@ -8,8 +8,8 @@ export { ListSchema, CommentsArray, CreateCommentsArray, - Comments, - CreateComments, + Comment, + CreateComment, ExceptionListSchema, ExceptionListItemSchema, CreateExceptionListSchema, @@ -30,6 +30,7 @@ export { ExceptionListTypeEnum, exceptionListItemSchema, exceptionListType, + comment, createExceptionListItemSchema, listSchema, entry, diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_comments.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_comments.tsx index db2d0540971de..22d14ec6bedb1 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_comments.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_comments.tsx @@ -16,13 +16,13 @@ import { EuiCommentProps, EuiText, } from '@elastic/eui'; -import { Comments } from '../../../lists_plugin_deps'; +import { Comment } from '../../../shared_imports'; import * as i18n from './translations'; import { useCurrentUser } from '../../lib/kibana'; import { getFormattedComments } from './helpers'; interface AddExceptionCommentsProps { - exceptionItemComments?: Comments[]; + exceptionItemComments?: Comment[]; newCommentValue: string; newCommentOnChange: (value: string) => void; } diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx index a4fe52eaacf4e..0f7e5b24ed8f9 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx @@ -38,7 +38,7 @@ import { useSignalIndex } from '../../../../detections/containers/detection_engi import { useFetchOrCreateRuleExceptionList } from '../use_fetch_or_create_rule_exception_list'; import { AddExceptionComments } from '../add_exception_comments'; import { - enrichExceptionItemsWithComments, + enrichNewExceptionItemsWithComments, enrichExceptionItemsWithOS, defaultEndpointExceptionItems, entryHasListType, @@ -251,7 +251,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({ let enriched: Array = []; enriched = comment !== '' - ? enrichExceptionItemsWithComments(exceptionItemsToAdd, [{ comment }]) + ? enrichNewExceptionItemsWithComments(exceptionItemsToAdd, [{ comment }]) : exceptionItemsToAdd; if (exceptionListType === 'endpoint') { const osTypes = retrieveAlertOsTypes(); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx index 1ec49425ce8fd..734434484fb4c 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx @@ -392,7 +392,7 @@ export const ExceptionBuilder = ({ )} { - addError(error, { title: i18n.EDIT_EXCEPTION_ERROR }); - onCancel(); + if (error.message.includes('Conflict')) { + setHasVersionConflict(true); + } else { + addError(error, { title: i18n.EDIT_EXCEPTION_ERROR }); + onCancel(); + } }, [addError, onCancel] ); @@ -147,8 +153,8 @@ export const EditExceptionModal = memo(function EditExceptionModal({ }, [shouldDisableBulkClose]); const isSubmitButtonDisabled = useMemo( - () => exceptionItemsToAdd.every((item) => item.entries.length === 0), - [exceptionItemsToAdd] + () => exceptionItemsToAdd.every((item) => item.entries.length === 0) || hasVersionConflict, + [exceptionItemsToAdd, hasVersionConflict] ); const handleBuilderOnChange = useCallback( @@ -177,11 +183,15 @@ export const EditExceptionModal = memo(function EditExceptionModal({ ); const enrichExceptionItems = useCallback(() => { - let enriched: Array = []; - enriched = enrichExceptionItemsWithComments(exceptionItemsToAdd, [ - ...(exceptionItem.comments ? exceptionItem.comments : []), - ...(comment !== '' ? [{ comment }] : []), - ]); + const [exceptionItemToEdit] = exceptionItemsToAdd; + let enriched: Array = [ + { + ...enrichExistingExceptionItemWithComments(exceptionItemToEdit, [ + ...exceptionItem.comments, + ...(comment.trim() !== '' ? [{ comment }] : []), + ]), + }, + ]; if (exceptionListType === 'endpoint') { const osTypes = exceptionItem._tags ? getOperatingSystems(exceptionItem._tags) : []; enriched = enrichExceptionItemsWithOS(enriched, osTypes); @@ -222,7 +232,7 @@ export const EditExceptionModal = memo(function EditExceptionModal({ listId={exceptionItem.list_id} listNamespaceType={exceptionItem.namespace_type} ruleName={ruleName} - isOrDisabled={false} + isOrDisabled isAndDisabled={false} isNestedDisabled={false} data-test-subj="edit-exception-modal-builder" @@ -263,6 +273,14 @@ export const EditExceptionModal = memo(function EditExceptionModal({ )} + {hasVersionConflict && ( + + +

{i18n.VERSION_CONFLICT_ERROR_DESCRIPTION}

+ + + )} + {i18n.CANCEL} diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/translations.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/translations.ts index 6c5cb733b7a73..d09f0158b2e1d 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/translations.ts @@ -67,3 +67,18 @@ export const EXCEPTION_BUILDER_INFO = i18n.translate( defaultMessage: "Alerts are generated when the rule's conditions are met, except when:", } ); + +export const VERSION_CONFLICT_ERROR_TITLE = i18n.translate( + 'xpack.securitySolution.exceptions.editException.versionConflictTitle', + { + defaultMessage: 'Sorry, there was an error', + } +); + +export const VERSION_CONFLICT_ERROR_DESCRIPTION = i18n.translate( + 'xpack.securitySolution.exceptions.editException.versionConflictDescription', + { + defaultMessage: + "It appears this exception was updated since you first selected to edit it. Try clicking 'Cancel' and editing the exception again.", + } +); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx index 78936d5d0da6f..5cb65ee6db8ff 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx @@ -18,7 +18,8 @@ import { formatOperatingSystems, getEntryValue, formatExceptionItemForUpdate, - enrichExceptionItemsWithComments, + enrichNewExceptionItemsWithComments, + enrichExistingExceptionItemWithComments, enrichExceptionItemsWithOS, entryHasListType, entryHasNonEcsType, @@ -35,14 +36,14 @@ import { existsOperator, doesNotExistOperator, } from '../autocomplete/operators'; -import { OperatorTypeEnum, OperatorEnum, EntryNested } from '../../../lists_plugin_deps'; +import { OperatorTypeEnum, OperatorEnum, EntryNested } from '../../../shared_imports'; import { getExceptionListItemSchemaMock } from '../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; import { getEntryMatchMock } from '../../../../../lists/common/schemas/types/entry_match.mock'; import { getEntryMatchAnyMock } from '../../../../../lists/common/schemas/types/entry_match_any.mock'; import { getEntryExistsMock } from '../../../../../lists/common/schemas/types/entry_exists.mock'; import { getEntryListMock } from '../../../../../lists/common/schemas/types/entry_list.mock'; -import { getCommentsArrayMock } from '../../../../../lists/common/schemas/types/comments.mock'; -import { ENTRIES } from '../../../../../lists/common/constants.mock'; +import { getCommentsArrayMock } from '../../../../../lists/common/schemas/types/comment.mock'; +import { ENTRIES, OLD_DATE_RELATIVE_TO_DATE_NOW } from '../../../../../lists/common/constants.mock'; import { CreateExceptionListItemSchema, ExceptionListItemSchema, @@ -410,12 +411,52 @@ describe('Exception helpers', () => { expect(result).toEqual(expected); }); }); + describe('#enrichExistingExceptionItemWithComments', () => { + test('it should return exception item with comments stripped of "created_by", "created_at", "updated_by", "updated_at" fields', () => { + const payload = getExceptionListItemSchemaMock(); + const comments = [ + { + comment: 'Im an existing comment', + created_at: OLD_DATE_RELATIVE_TO_DATE_NOW, + created_by: 'lily', + id: '1', + }, + { + comment: 'Im another existing comment', + created_at: OLD_DATE_RELATIVE_TO_DATE_NOW, + created_by: 'lily', + id: '2', + }, + { + comment: 'Im a new comment', + }, + ]; + const result = enrichExistingExceptionItemWithComments(payload, comments); + const expected = { + ...getExceptionListItemSchemaMock(), + comments: [ + { + comment: 'Im an existing comment', + id: '1', + }, + { + comment: 'Im another existing comment', + id: '2', + }, + { + comment: 'Im a new comment', + }, + ], + }; + expect(result).toEqual(expected); + }); + }); - describe('#enrichExceptionItemsWithComments', () => { + describe('#enrichNewExceptionItemsWithComments', () => { test('it should add comments to an exception item', () => { const payload = [getExceptionListItemSchemaMock()]; const comments = getCommentsArrayMock(); - const result = enrichExceptionItemsWithComments(payload, comments); + const result = enrichNewExceptionItemsWithComments(payload, comments); const expected = [ { ...getExceptionListItemSchemaMock(), @@ -428,7 +469,7 @@ describe('Exception helpers', () => { test('it should add comments to multiple exception items', () => { const payload = [getExceptionListItemSchemaMock(), getExceptionListItemSchemaMock()]; const comments = getCommentsArrayMock(); - const result = enrichExceptionItemsWithComments(payload, comments); + const result = enrichNewExceptionItemsWithComments(payload, comments); const expected = [ { ...getExceptionListItemSchemaMock(), diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx index a54f20f56d56f..ee45f9b5de1fa 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx @@ -20,13 +20,14 @@ import { EXCEPTION_OPERATORS, isOperator } from '../autocomplete/operators'; import { OperatorOption } from '../autocomplete/types'; import { CommentsArray, - Comments, - CreateComments, + Comment, + CreateComment, Entry, ExceptionListItemSchema, NamespaceType, OperatorTypeEnum, CreateExceptionListItemSchema, + comment, entry, entriesNested, createExceptionListItemSchema, @@ -34,7 +35,7 @@ import { UpdateExceptionListItemSchema, ExceptionListType, EntryNested, -} from '../../../lists_plugin_deps'; +} from '../../../shared_imports'; import { IIndexPattern } from '../../../../../../../src/plugins/data/common'; import { validate } from '../../../../common/validate'; import { TimelineNonEcsData } from '../../../graphql/types'; @@ -140,16 +141,16 @@ export const getTagsInclude = ({ * @param comments ExceptionItem.comments */ export const getFormattedComments = (comments: CommentsArray): EuiCommentProps[] => - comments.map((comment) => ({ - username: comment.created_by, - timestamp: moment(comment.created_at).format('on MMM Do YYYY @ HH:mm:ss'), + comments.map((commentItem) => ({ + username: commentItem.created_by, + timestamp: moment(commentItem.created_at).format('on MMM Do YYYY @ HH:mm:ss'), event: i18n.COMMENT_EVENT, - timelineIcon: , - children: {comment.comment}, + timelineIcon: , + children: {commentItem.comment}, actions: ( ), @@ -271,11 +272,11 @@ export const prepareExceptionItemsForBulkClose = ( /** * Adds new and existing comments to all new exceptionItems if not present already * @param exceptionItems new or existing ExceptionItem[] - * @param comments new Comments + * @param comments new Comment */ -export const enrichExceptionItemsWithComments = ( +export const enrichNewExceptionItemsWithComments = ( exceptionItems: Array, - comments: Array + comments: Array ): Array => { return exceptionItems.map((item: ExceptionListItemSchema | CreateExceptionListItemSchema) => { return { @@ -285,6 +286,36 @@ export const enrichExceptionItemsWithComments = ( }); }; +/** + * Adds new and existing comments to exceptionItem + * @param exceptionItem existing ExceptionItem + * @param comments array of comments that can include existing + * and new comments + */ +export const enrichExistingExceptionItemWithComments = ( + exceptionItem: ExceptionListItemSchema | CreateExceptionListItemSchema, + comments: Array +): ExceptionListItemSchema | CreateExceptionListItemSchema => { + const formattedComments = comments.map((item) => { + if (comment.is(item)) { + const { id, comment: existingComment } = item; + return { + id, + comment: existingComment, + }; + } else { + return { + comment: item.comment, + }; + } + }); + + return { + ...exceptionItem, + comments: formattedComments, + }; +}; + /** * Adds provided osTypes to all exceptionItems if not present already * @param exceptionItems new or existing ExceptionItem[] diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_details.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_details.test.tsx index 8df7b51bb9d31..ab6588b67d5ba 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_details.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_details.test.tsx @@ -12,7 +12,7 @@ import moment from 'moment-timezone'; import { ExceptionDetails } from './exception_details'; import { getExceptionListItemSchemaMock } from '../../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; -import { getCommentsArrayMock } from '../../../../../../../lists/common/schemas/types/comments.mock'; +import { getCommentsArrayMock } from '../../../../../../../lists/common/schemas/types/comment.mock'; describe('ExceptionDetails', () => { beforeEach(() => { diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.stories.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.stories.tsx index 56b029aaee81e..fec7354855935 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.stories.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.stories.tsx @@ -11,7 +11,7 @@ import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; import { ExceptionItem } from './'; import { getExceptionListItemSchemaMock } from '../../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; -import { getCommentsArrayMock } from '../../../../../../../lists/common/schemas/types/comments.mock'; +import { getCommentsArrayMock } from '../../../../../../../lists/common/schemas/types/comment.mock'; addDecorator((storyFn) => ( ({ eui: euiLightVars, darkMode: false })}>{storyFn()} diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.test.tsx index 90752f9450e4c..c9def092fda47 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.test.tsx @@ -11,7 +11,7 @@ import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; import { ExceptionItem } from './'; import { getExceptionListItemSchemaMock } from '../../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; -import { getCommentsArrayMock } from '../../../../../../../lists/common/schemas/types/comments.mock'; +import { getCommentsArrayMock } from '../../../../../../../lists/common/schemas/types/comment.mock'; jest.mock('../../../../lib/kibana'); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx index 34dc47b9cd411..16eaef4136983 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx @@ -190,7 +190,8 @@ const ExceptionsViewerComponent = ({ const handleOnCancelExceptionModal = useCallback((): void => { setCurrentModal(null); - }, [setCurrentModal]); + handleFetchList(); + }, [setCurrentModal, handleFetchList]); const handleOnConfirmExceptionModal = useCallback((): void => { setCurrentModal(null);