Skip to content

Commit

Permalink
[SIEM][Exceptions] - Cleaned up and updated exception list item comme…
Browse files Browse the repository at this point in the history 
…nt structure (#69532) (#70107)

### Summary

This PR is a follow up to #68864 . That PR used a partial to differentiate between new and existing comments, this meant that comments could be updated when they shouldn't. It was decided in our discussion of exception list schemas that comments should be append only. This PR assures that's the case, but also leaves it open to editing comments (via API). It checks to make sure that users can only update their own comments.
  • Loading branch information
@yctercero
yctercero authored Jun 26, 2020
1 parent 14699f7 commit 936af29
Show file tree
Hide file tree
Showing 35 changed files with 1,437 additions and 137 deletions.
54 changes: 42 additions & 12 deletions x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ 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 { CommentsArray } from '../types';

import {
CreateExceptionListItemSchema,
Expand All @@ -26,7 +29,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(payload);
});

test('it should not accept an undefined for "description"', () => {
test('it should not validate an undefined for "description"', () => {
const payload = getCreateExceptionListItemSchemaMock();
delete payload.description;
const decoded = createExceptionListItemSchema.decode(payload);
Expand All @@ -38,7 +41,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual({});
});

test('it should not accept an undefined for "name"', () => {
test('it should not validate an undefined for "name"', () => {
const payload = getCreateExceptionListItemSchemaMock();
delete payload.name;
const decoded = createExceptionListItemSchema.decode(payload);
Expand All @@ -50,7 +53,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual({});
});

test('it should not accept an undefined for "type"', () => {
test('it should not validate an undefined for "type"', () => {
const payload = getCreateExceptionListItemSchemaMock();
delete payload.type;
const decoded = createExceptionListItemSchema.decode(payload);
Expand All @@ -62,7 +65,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual({});
});

test('it should not accept an undefined for "list_id"', () => {
test('it should not validate an undefined for "list_id"', () => {
const inputPayload = getCreateExceptionListItemSchemaMock();
delete inputPayload.list_id;
const decoded = createExceptionListItemSchema.decode(inputPayload);
Expand All @@ -74,7 +77,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual({});
});

test('it should accept an undefined for "meta" but strip it out and generate a correct body not counting the auto generated uuid', () => {
test('it should validate 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 @@ -87,7 +90,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(outputPayload);
});

test('it should accept an undefined for "comments" but return an array and generate a correct body not counting the auto generated uuid', () => {
test('it should validate 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 @@ -100,7 +103,34 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(outputPayload);
});

test('it should accept an undefined for "entries" but return an array', () => {
test('it should validate "comments" array', () => {
const inputPayload = {
...getCreateExceptionListItemSchemaMock(),
comments: getCreateCommentsArrayMock(),
};
const decoded = createExceptionListItemSchema.decode(inputPayload);
const checked = exactCheck(inputPayload, decoded);
const message = pipe(checked, foldLeftRight);
delete (message.schema as CreateExceptionListItemSchema).item_id;
expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(inputPayload);
});

test('it should NOT validate "comments" with "created_at" or "created_by" values', () => {
const inputPayload: Omit<CreateExceptionListItemSchema, 'comments'> & {
comments?: CommentsArray;
} = {
...getCreateExceptionListItemSchemaMock(),
comments: [getCommentsMock()],
};
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(message.schema).toEqual({});
});

test('it should validate an undefined for "entries" but return an array', () => {
const inputPayload = getCreateExceptionListItemSchemaMock();
const outputPayload = getCreateExceptionListItemSchemaMock();
delete inputPayload.entries;
Expand All @@ -113,7 +143,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(outputPayload);
});

test('it should accept an undefined for "namespace_type" but return enum "single" and generate a correct body not counting the auto generated uuid', () => {
test('it should validate 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 @@ -126,7 +156,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(outputPayload);
});

test('it should accept an undefined for "tags" but return an array and generate a correct body not counting the auto generated uuid', () => {
test('it should validate 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 @@ -139,7 +169,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(outputPayload);
});

test('it should accept an undefined for "_tags" but return an array and generate a correct body not counting the auto generated uuid', () => {
test('it should validate 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 @@ -152,7 +182,7 @@ describe('create_exception_list_item_schema', () => {
expect(message.schema).toEqual(outputPayload);
});

test('it should accept an undefined for "item_id" and auto generate a uuid', () => {
test('it should validate an undefined for "item_id" and auto generate a uuid', () => {
const inputPayload = getCreateExceptionListItemSchemaMock();
delete inputPayload.item_id;
const decoded = createExceptionListItemSchema.decode(inputPayload);
Expand All @@ -164,7 +194,7 @@ describe('create_exception_list_item_schema', () => {
);
});

test('it should accept an undefined for "item_id" and generate a correct body not counting the uuid', () => {
test('it should validate 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
6 changes: 3 additions & 3 deletions x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
tags,
} from '../common/schemas';
import { Identity, RequiredKeepUndefined } from '../../types';
import { CommentsPartialArray, DefaultCommentsPartialArray, DefaultEntryArray } from '../types';
import { CreateCommentsArray, DefaultCreateCommentsArray, DefaultEntryArray } from '../types';
import { EntriesArray } from '../types/entries';
import { DefaultUuid } from '../../siem_common_deps';

Expand All @@ -39,7 +39,7 @@ export const createExceptionListItemSchema = t.intersection([
t.exact(
t.partial({
_tags, // defaults to empty array if not set during decode
comments: DefaultCommentsPartialArray, // defaults to empty array if not set during decode
comments: DefaultCreateCommentsArray, // defaults to empty array if not set during decode
entries: DefaultEntryArray, // defaults to empty array if not set during decode
item_id: DefaultUuid, // defaults to GUID (uuid v4) if not set during decode
meta, // defaults to undefined if not set during decode
Expand All @@ -63,7 +63,7 @@ export type CreateExceptionListItemSchemaDecoded = Identity<
'_tags' | 'tags' | 'item_id' | 'entries' | 'namespace_type' | 'comments'
> & {
_tags: _Tags;
comments: CommentsPartialArray;
comments: CreateCommentsArray;
tags: Tags;
item_id: ItemId;
entries: EntriesArray;
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/lists/common/schemas/request/update_exception_list_item_schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import {
} from '../common/schemas';
import { Identity, RequiredKeepUndefined } from '../../types';
import {
CommentsPartialArray,
DefaultCommentsPartialArray,
DefaultEntryArray,
DefaultUpdateCommentsArray,
EntriesArray,
UpdateCommentsArray,
} from '../types';

export const updateExceptionListItemSchema = t.intersection([
Expand All @@ -40,7 +40,7 @@ export const updateExceptionListItemSchema = t.intersection([
t.exact(
t.partial({
_tags, // defaults to empty array if not set during decode
comments: DefaultCommentsPartialArray, // defaults to empty array if not set during decode
comments: DefaultUpdateCommentsArray, // defaults to empty array if not set during decode
entries: DefaultEntryArray, // defaults to empty array if not set during decode
id, // defaults to undefined if not set during decode
item_id: t.union([t.string, t.undefined]),
Expand All @@ -65,7 +65,7 @@ export type UpdateExceptionListItemSchemaDecoded = Identity<
'_tags' | 'tags' | 'entries' | 'namespace_type' | 'comments'
> & {
_tags: _Tags;
comments: CommentsPartialArray;
comments: UpdateCommentsArray;
tags: Tags;
entries: EntriesArray;
namespace_type: NamespaceType;
Expand Down
21 changes: 8 additions & 13 deletions x-pack/plugins/lists/common/schemas/types/comments.mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,12 @@

import { DATE_NOW, USER } from '../../constants.mock';

import { CommentsArray } from './comments';
import { Comments, CommentsArray } from './comments';

export const getCommentsMock = (): CommentsArray => [
{
comment: 'some comment',
created_at: DATE_NOW,
created_by: USER,
},
{
comment: 'some other comment',
created_at: DATE_NOW,
created_by: 'lily',
},
];
export const getCommentsMock = (): Comments => ({
comment: 'some old comment',
created_at: DATE_NOW,
created_by: USER,
});

export const getCommentsArrayMock = (): CommentsArray => [getCommentsMock(), getCommentsMock()];
Loading

0 comments on commit 936af29

Please sign in to comment.