Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Exceptions] - Tie server and client code together #70918

Merged
merged 8 commits into from
Jul 7, 2020
33 changes: 32 additions & 1 deletion x-pack/plugins/lists/common/schemas/common/schemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { left } from 'fp-ts/lib/Either';

import { foldLeftRight, getPaths } from '../../siem_common_deps';

import { operator, operator_type as operatorType } from './schemas';
import { exceptionListType, operator, operator_type as operatorType } from './schemas';

describe('Common schemas', () => {
describe('operatorType', () => {
Expand Down Expand Up @@ -91,4 +91,35 @@ describe('Common schemas', () => {
expect(keys.length).toEqual(2);
});
});

describe('exceptionListType', () => {
test('it should validate for "detection"', () => {
const payload = 'detection';
const decoded = exceptionListType.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(payload);
});

test('it should validate for "endpoint"', () => {
const payload = 'endpoint';
const decoded = exceptionListType.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(payload);
});

test('it should contain 2 keys', () => {
// Might seem like a weird test, but its meant to
// ensure that if exceptionListType is updated, you
// also update the ExceptionListTypeEnum, a workaround
// for io-ts not yet supporting enums
// https://github.com/gcanti/io-ts/issues/67
const keys = Object.keys(exceptionListType.keys);

expect(keys.length).toEqual(2);
});
});
});
12 changes: 8 additions & 4 deletions x-pack/plugins/lists/common/schemas/common/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,19 @@ export type _Tags = t.TypeOf<typeof _tags>;
export const _tagsOrUndefined = t.union([_tags, t.undefined]);
export type _TagsOrUndefined = t.TypeOf<typeof _tagsOrUndefined>;

// TODO: Change this into a t.keyof enumeration when we know what types of lists we going to have.
export const exceptionListType = t.string;
export const exceptionListType = t.keyof({ detection: null, endpoint: null });
export const exceptionListTypeOrUndefined = t.union([exceptionListType, t.undefined]);
export type ExceptionListType = t.TypeOf<typeof exceptionListType>;
export type ExceptionListTypeOrUndefined = t.TypeOf<typeof exceptionListTypeOrUndefined>;
export enum ExceptionListTypeEnum {
DETECTION = 'detection',
ENDPOINT = 'endpoint',
}

// TODO: Change this into a t.keyof enumeration when we know what types of lists we going to have.
export const exceptionListItemType = t.string;
export const exceptionListItemType = t.keyof({ simple: null });
export const exceptionListItemTypeOrUndefined = t.union([exceptionListItemType, t.undefined]);
export type ExceptionListItemType = t.TypeOf<typeof exceptionListItemType>;
export type ExceptionListItemTypeOrUndefined = t.TypeOf<typeof exceptionListItemTypeOrUndefined>;

export const list_type = t.keyof({ item: null, list: null });
export type ListType = t.TypeOf<typeof list_type>;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/lists/public/exceptions/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ describe('Exceptions Lists API', () => {
});

test('it returns error if response payload fails decode', async () => {
const badPayload = getExceptionListItemSchemaMock();
const badPayload = getExceptionListSchemaMock();
delete badPayload.id;
fetchMock.mockResolvedValue(badPayload);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('useExceptionList', () => {
useExceptionList({
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
lists: [{ id: 'myListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
pagination: {
page: 1,
Expand Down Expand Up @@ -76,7 +76,7 @@ describe('useExceptionList', () => {
useExceptionList({
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
lists: [{ id: 'myListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
onSuccess: onSuccessMock,
pagination: {
Expand Down Expand Up @@ -131,7 +131,7 @@ describe('useExceptionList', () => {
initialProps: {
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
lists: [{ id: 'myListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
onSuccess: onSuccessMock,
pagination: {
Expand All @@ -146,7 +146,7 @@ describe('useExceptionList', () => {
rerender({
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'newListId', namespaceType: 'single' }],
lists: [{ id: 'newListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
onSuccess: onSuccessMock,
pagination: {
Expand All @@ -173,7 +173,7 @@ describe('useExceptionList', () => {
useExceptionList({
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
lists: [{ id: 'myListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
pagination: {
page: 1,
Expand Down Expand Up @@ -210,7 +210,7 @@ describe('useExceptionList', () => {
useExceptionList({
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
lists: [{ id: 'myListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
pagination: {
page: 1,
Expand Down Expand Up @@ -238,7 +238,7 @@ describe('useExceptionList', () => {
useExceptionList({
filterOptions: { filter: '', tags: [] },
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
lists: [{ id: 'myListId', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
pagination: {
page: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useEffect, useMemo, useRef, useState } from 'react';

import { fetchExceptionListById, fetchExceptionListItemsByListId } from '../api';
import { ExceptionIdentifiers, ExceptionList, Pagination, UseExceptionListProps } from '../types';
import { ExceptionListItemSchema } from '../../../common/schemas';
import { ExceptionListItemSchema, NamespaceType } from '../../../common/schemas';

type Func = () => void;
export type ReturnExceptionListAndItems = [
Expand Down Expand Up @@ -73,7 +73,13 @@ export const useExceptionList = ({
let exceptions: ExceptionListItemSchema[] = [];
let exceptionListsReturned: ExceptionList[] = [];

const fetchData = async ({ id, namespaceType }: ExceptionIdentifiers): Promise<void> => {
const fetchData = async ({
id,
namespaceType,
}: {
id: string;
namespaceType: NamespaceType;
}): Promise<void> => {
try {
setLoading(true);

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/lists/public/exceptions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
CreateExceptionListSchema,
ExceptionListItemSchema,
ExceptionListSchema,
ExceptionListType,
NamespaceType,
Page,
PerPage,
Expand Down Expand Up @@ -60,7 +61,7 @@ export interface UseExceptionListProps {
export interface ExceptionIdentifiers {
id: string;
namespaceType: NamespaceType;
type?: string;
type: ExceptionListType;
}

export interface ApiCallByListIdProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"field": "event.module",
"operator": "excluded",
"type": "match_any",
"value": ["zeek"]
"value": ["suricata"]
},
{
"field": "source.ip",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"id": "hand_inserted_item_id",
"list_id": "list-ip",
"value": "10.4.2.140"
"value": "10.4.3.11"
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import {
Description,
EntriesArray,
ExceptionListItemSchema,
ExceptionListItemType,
ExceptionListSoSchema,
ExceptionListType,
ItemId,
ListId,
MetaOrUndefined,
Expand Down Expand Up @@ -43,7 +43,7 @@ interface CreateExceptionListItemOptions {
user: string;
tags: Tags;
tieBreaker?: string;
type: ExceptionListType;
type: ExceptionListItemType;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before both the list and item types were just t.string so it didn't scream at us, but now that the list type and item type differ, had to update.

}

export const createExceptionListItem = async ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
DescriptionOrUndefined,
EntriesArray,
EntriesArrayOrUndefined,
ExceptionListItemType,
ExceptionListItemTypeOrUndefined,
ExceptionListType,
ExceptionListTypeOrUndefined,
FilterOrUndefined,
Expand Down Expand Up @@ -98,7 +100,7 @@ export interface CreateExceptionListItemOptions {
description: Description;
meta: MetaOrUndefined;
tags: Tags;
type: ExceptionListType;
type: ExceptionListItemType;
}

export interface UpdateExceptionListItemOptions {
Expand All @@ -112,7 +114,7 @@ export interface UpdateExceptionListItemOptions {
description: DescriptionOrUndefined;
meta: MetaOrUndefined;
tags: TagsOrUndefined;
type: ExceptionListTypeOrUndefined;
type: ExceptionListItemTypeOrUndefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before both the list and item types were just t.string so it didn't scream at us, but now that the list type and item type differ, had to update.

}

export interface FindExceptionListItemOptions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import {
DescriptionOrUndefined,
EntriesArrayOrUndefined,
ExceptionListItemSchema,
ExceptionListItemTypeOrUndefined,
ExceptionListSoSchema,
ExceptionListTypeOrUndefined,
IdOrUndefined,
ItemIdOrUndefined,
MetaOrUndefined,
Expand Down Expand Up @@ -43,7 +43,7 @@ interface UpdateExceptionListItemOptions {
user: string;
tags: TagsOrUndefined;
tieBreaker?: string;
type: ExceptionListTypeOrUndefined;
type: ExceptionListItemTypeOrUndefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before both the list and item types were just t.string so it didn't scream at us, but now that the list type and item type differ, had to update.

}

export const updateExceptionListItem = async ({
Expand Down
12 changes: 8 additions & 4 deletions x-pack/plugins/lists/server/services/exception_lists/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {
NamespaceType,
UpdateCommentsArrayOrUndefined,
comments as commentsSchema,
exceptionListItemType,
exceptionListType,
} from '../../../common/schemas';
import {
SavedObjectType,
Expand Down Expand Up @@ -80,7 +82,7 @@ export const transformSavedObjectToExceptionList = ({
namespace_type: namespaceType,
tags,
tie_breaker_id,
type,
type: exceptionListType.is(type) ? type : 'detection',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a result of both the list and item type being mapped to the same type property in the SO. We can discuss whether to keep this as is (@FrankHassanabad are there downsides to separating these out?) or change.

updated_at: updatedAt ?? dateNow,
updated_by,
};
Expand Down Expand Up @@ -116,7 +118,7 @@ export const transformSavedObjectUpdateToExceptionList = ({
namespace_type: namespaceType,
tags: tags ?? exceptionList.tags,
tie_breaker_id: exceptionList.tie_breaker_id,
type: type ?? exceptionList.type,
type: exceptionListType.is(type) ? type : exceptionList.type,
updated_at: updatedAt ?? dateNow,
updated_by: updatedBy ?? exceptionList.updated_by,
};
Expand Down Expand Up @@ -168,7 +170,7 @@ export const transformSavedObjectToExceptionListItem = ({
namespace_type: namespaceType,
tags,
tie_breaker_id,
type,
type: exceptionListItemType.is(type) ? type : 'simple',
updated_at: updatedAt ?? dateNow,
updated_by,
};
Expand Down Expand Up @@ -202,6 +204,8 @@ export const transformSavedObjectUpdateToExceptionListItem = ({

// TODO: Change this to do a decode and throw if the saved object is not as expected.
// TODO: Do a throw if after the decode this is not the correct "list_type: list"
// TODO: Update exception list and item types (perhaps separating out) so as to avoid
// defaulting
return {
_tags: _tags ?? exceptionListItem._tags,
comments: comments ?? exceptionListItem.comments,
Expand All @@ -217,7 +221,7 @@ export const transformSavedObjectUpdateToExceptionListItem = ({
namespace_type: namespaceType,
tags: tags ?? exceptionListItem.tags,
tie_breaker_id: exceptionListItem.tie_breaker_id,
type: type ?? exceptionListItem.type,
type: exceptionListItemType.is(type) ? type : exceptionListItem.type,
updated_at: updatedAt ?? dateNow,
updated_by: updatedBy ?? exceptionListItem.updated_by,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
entriesExists,
entriesMatch,
entriesNested,
entriesList,
ExceptionListItemSchema,
} from '../../../lists/common/schemas';
import { Language, Query } from './schemas/common/schemas';
Expand Down Expand Up @@ -182,7 +181,7 @@ export const buildExceptionItemEntries = ({
}): string => {
const and = getLanguageBooleanOperator({ language, value: 'and' });
const exceptionItem = lists
.filter((t) => !entriesList.is(t))
.filter(({ type }) => type !== 'list')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to try to not rely on io-ts check where I can given talk of not optimal performance.

.reduce<string[]>((accum, listItem) => {
const exceptionSegment = evaluateValues({ item: listItem, language });
return [...accum, exceptionSegment];
Expand All @@ -200,7 +199,7 @@ export const buildQueryExceptions = ({
language: Language;
lists: ExceptionListItemSchema[] | undefined;
}): DataQuery[] => {
if (lists && lists !== null) {
if (lists != null) {
const exceptions = lists.map((exceptionItem) =>
buildExceptionItemEntries({ lists: exceptionItem.entries, language })
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { EntriesArray, namespaceType } from '../../../lists/common/schemas';
export { EntriesArray, exceptionListType, namespaceType } from '../../../lists/common/schemas';
Original file line number Diff line number Diff line change
Expand Up @@ -1447,10 +1447,12 @@ describe('add prepackaged rules schema', () => {
{
id: 'some_uuid',
namespace_type: 'single',
type: 'detection',
},
{
id: 'some_uuid',
namespace_type: 'agnostic',
type: 'endpoint',
},
],
};
Expand Down Expand Up @@ -1533,6 +1535,7 @@ describe('add prepackaged rules schema', () => {
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "undefined" supplied to "exceptions_list,type"',
'Invalid value "not a namespace type" supplied to "exceptions_list,namespace_type"',
]);
expect(message.schema).toEqual({});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1514,10 +1514,12 @@ describe('create rules schema', () => {
{
id: 'some_uuid',
namespace_type: 'single',
type: 'detection',
},
{
id: 'some_uuid',
namespace_type: 'agnostic',
type: 'endpoint',
},
],
};
Expand Down Expand Up @@ -1598,6 +1600,7 @@ describe('create rules schema', () => {
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "undefined" supplied to "exceptions_list,type"',
'Invalid value "not a namespace type" supplied to "exceptions_list,namespace_type"',
]);
expect(message.schema).toEqual({});
Expand Down