[Security Solution][Exceptions] - Update exception item comments to include id (#73129) (#73375)

## 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

Author: yctercero

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';

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<CreateEndpointListItemSchema, 'comments'> & {
       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);

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<CreateExceptionListItemSchema, 'comments'> & {
       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);

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

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