Skip to content

Commit

Permalink
[Security Solution][Exceptions] - Update exception item comments to i…
Browse files Browse the repository at this point in the history
…nclude 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
  • Loading branch information
yctercero authored Jul 27, 2020
1 parent 57997be commit 94ed783
Show file tree
Hide file tree
Showing 41 changed files with 702 additions and 783 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/lists/common/constants.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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(),
Expand All @@ -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;
} = {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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(),
Expand All @@ -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;
} = {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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']);
});
});
});
Original file line number Diff line number Diff line change
@@ -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)];
};
Loading

0 comments on commit 94ed783

Please sign in to comment.