Skip to content

Commit

Permalink
[Security Solution][Exceptions] - Updates exception hooks and viewer (#…
Browse files Browse the repository at this point in the history 
…73588)

## Summary

This PR focuses on addressing issues around the pagination and functionality of rules with numerous (2+) exception lists.

- Updated the `use_exception_list.ts` hook to make use of the new multi list find API
- Updated the viewer to make use of the new multi list find API
  - Previously was doing a lot of the filtering and paging manually (and badly) in the UI, now the _find takes care of all that
- Added logic for showing `No results` text if user filter/search returns no items
  - Previously would show the `This rule has not exceptions` text
  • Loading branch information
@yctercero
yctercero authored Jul 29, 2020
1 parent c102c4f commit 0756dd3
Show file tree
Hide file tree
Showing 23 changed files with 1,114 additions and 423 deletions.
102 changes: 57 additions & 45 deletions x-pack/plugins/lists/public/exceptions/api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
deleteExceptionListItemById,
fetchExceptionListById,
fetchExceptionListItemById,
fetchExceptionListItemsByListId,
fetchExceptionListsItemsByListIds,
updateExceptionList,
updateExceptionListItem,
} from './api';
Expand Down Expand Up @@ -358,17 +358,18 @@ describe('Exceptions Lists API', () => {
});
});

describe('#fetchExceptionListItemsByListId', () => {
describe('#fetchExceptionListsItemsByListIds', () => {
beforeEach(() => {
fetchMock.mockClear();
fetchMock.mockResolvedValue(getFoundExceptionListItemSchemaMock());
});

test('it invokes "fetchExceptionListItemsByListId" with expected url and body values', async () => {
await fetchExceptionListItemsByListId({
test('it invokes "fetchExceptionListsItemsByListIds" with expected url and body values', async () => {
await fetchExceptionListsItemsByListIds({
filterOptions: [],
http: mockKibanaHttpService(),
listId: 'myList',
namespaceType: 'single',
listIds: ['myList', 'myOtherListId'],
namespaceTypes: ['single', 'single'],
pagination: {
page: 1,
perPage: 20,
Expand All @@ -379,8 +380,8 @@ describe('Exceptions Lists API', () => {
expect(fetchMock).toHaveBeenCalledWith('/api/exception_lists/items/_find', {
method: 'GET',
query: {
list_id: 'myList',
namespace_type: 'single',
list_id: 'myList,myOtherListId',
namespace_type: 'single,single',
page: '1',
per_page: '20',
},
Expand All @@ -389,14 +390,16 @@ describe('Exceptions Lists API', () => {
});

test('it invokes with expected url and body values when a filter exists and "namespaceType" of "single"', async () => {
await fetchExceptionListItemsByListId({
filterOptions: {
filter: 'hello world',
tags: [],
},
await fetchExceptionListsItemsByListIds({
filterOptions: [
{
filter: 'hello world',
tags: [],
},
],
http: mockKibanaHttpService(),
listId: 'myList',
namespaceType: 'single',
listIds: ['myList'],
namespaceTypes: ['single'],
pagination: {
page: 1,
perPage: 20,
Expand All @@ -418,14 +421,16 @@ describe('Exceptions Lists API', () => {
});

test('it invokes with expected url and body values when a filter exists and "namespaceType" of "agnostic"', async () => {
await fetchExceptionListItemsByListId({
filterOptions: {
filter: 'hello world',
tags: [],
},
await fetchExceptionListsItemsByListIds({
filterOptions: [
{
filter: 'hello world',
tags: [],
},
],
http: mockKibanaHttpService(),
listId: 'myList',
namespaceType: 'agnostic',
listIds: ['myList'],
namespaceTypes: ['agnostic'],
pagination: {
page: 1,
perPage: 20,
Expand All @@ -447,14 +452,16 @@ describe('Exceptions Lists API', () => {
});

test('it invokes with expected url and body values when tags exists', async () => {
await fetchExceptionListItemsByListId({
filterOptions: {
filter: '',
tags: ['malware'],
},
await fetchExceptionListsItemsByListIds({
filterOptions: [
{
filter: '',
tags: ['malware'],
},
],
http: mockKibanaHttpService(),
listId: 'myList',
namespaceType: 'agnostic',
listIds: ['myList'],
namespaceTypes: ['agnostic'],
pagination: {
page: 1,
perPage: 20,
Expand All @@ -476,14 +483,16 @@ describe('Exceptions Lists API', () => {
});

test('it invokes with expected url and body values when filter and tags exists', async () => {
await fetchExceptionListItemsByListId({
filterOptions: {
filter: 'host.name',
tags: ['malware'],
},
await fetchExceptionListsItemsByListIds({
filterOptions: [
{
filter: 'host.name',
tags: ['malware'],
},
],
http: mockKibanaHttpService(),
listId: 'myList',
namespaceType: 'agnostic',
listIds: ['myList'],
namespaceTypes: ['agnostic'],
pagination: {
page: 1,
perPage: 20,
Expand All @@ -506,10 +515,11 @@ describe('Exceptions Lists API', () => {
});

test('it returns expected format when call succeeds', async () => {
const exceptionResponse = await fetchExceptionListItemsByListId({
const exceptionResponse = await fetchExceptionListsItemsByListIds({
filterOptions: [],
http: mockKibanaHttpService(),
listId: 'endpoint_list_id',
namespaceType: 'single',
listIds: ['endpoint_list_id'],
namespaceTypes: ['single'],
pagination: {
page: 1,
perPage: 20,
Expand All @@ -521,16 +531,17 @@ describe('Exceptions Lists API', () => {

test('it returns error and does not make request if request payload fails decode', async () => {
const payload = ({
filterOptions: [],
http: mockKibanaHttpService(),
listId: '1',
namespaceType: 'not a namespace type',
listIds: ['myList'],
namespaceTypes: ['not a namespace type'],
pagination: {
page: 1,
perPage: 20,
},
signal: abortCtrl.signal,
} as unknown) as ApiCallByListIdProps & { listId: number };
await expect(fetchExceptionListItemsByListId(payload)).rejects.toEqual(
await expect(fetchExceptionListsItemsByListIds(payload)).rejects.toEqual(
'Invalid value "not a namespace type" supplied to "namespace_type"'
);
});
Expand All @@ -541,10 +552,11 @@ describe('Exceptions Lists API', () => {
fetchMock.mockResolvedValue(badPayload);

await expect(
fetchExceptionListItemsByListId({
fetchExceptionListsItemsByListIds({
filterOptions: [],
http: mockKibanaHttpService(),
listId: 'myList',
namespaceType: 'single',
listIds: ['myList'],
namespaceTypes: ['single'],
pagination: {
page: 1,
perPage: 20,
Expand Down
48 changes: 26 additions & 22 deletions x-pack/plugins/lists/public/exceptions/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,42 +249,46 @@ export const fetchExceptionListById = async ({
* Fetch an ExceptionList's ExceptionItems by providing a ExceptionList list_id
*
* @param http Kibana http service
* @param listId ExceptionList list_id (not ID)
* @param namespaceType ExceptionList namespace_type
* @param listIds ExceptionList list_ids (not ID)
* @param namespaceTypes ExceptionList namespace_types
* @param filterOptions optional - filter by field or tags
* @param pagination optional
* @param signal to cancel request
*
* @throws An error if response is not OK
*/
export const fetchExceptionListItemsByListId = async ({
export const fetchExceptionListsItemsByListIds = async ({
http,
listId,
namespaceType,
filterOptions = {
filter: '',
tags: [],
},
listIds,
namespaceTypes,
filterOptions,
pagination,
signal,
}: ApiCallByListIdProps): Promise<FoundExceptionListItemSchema> => {
const namespace =
namespaceType === 'agnostic' ? EXCEPTION_LIST_NAMESPACE_AGNOSTIC : EXCEPTION_LIST_NAMESPACE;
const filters = [
...(filterOptions.filter.length
? [`${namespace}.attributes.entries.field:${filterOptions.filter}*`]
: []),
...(filterOptions.tags.length
? filterOptions.tags.map((t) => `${namespace}.attributes.tags:${t}`)
: []),
];
const filters: string = filterOptions
.map<string>((filter, index) => {
const namespace = namespaceTypes[index];
const filterNamespace =
namespace === 'agnostic' ? EXCEPTION_LIST_NAMESPACE_AGNOSTIC : EXCEPTION_LIST_NAMESPACE;
const formattedFilters = [
...(filter.filter.length
? [`${filterNamespace}.attributes.entries.field:${filter.filter}*`]
: []),
...(filter.tags.length
? filter.tags.map((t) => `${filterNamespace}.attributes.tags:${t}`)
: []),
];

return formattedFilters.join(' AND ');
})
.join(',');

const query = {
list_id: listId,
namespace_type: namespaceType,
list_id: listIds.join(','),
namespace_type: namespaceTypes.join(','),
page: pagination.page ? `${pagination.page}` : '1',
per_page: pagination.perPage ? `${pagination.perPage}` : '20',
...(filters.length ? { filter: filters.join(' AND ') } : {}),
...(filters.trim() !== '' ? { filter: filters } : {}),
};
const [validatedRequest, errorsRequest] = validate(query, findExceptionListItemSchema);

Expand Down
115 changes: 114 additions & 1 deletion x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { act, renderHook } from '@testing-library/react-hooks';
import * as api from '../api';
import { createKibanaCoreStartMock } from '../../common/mocks/kibana_core';
import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock';
import { getFoundExceptionListItemSchemaMock } from '../../../common/schemas/response/found_exception_list_item_schema.mock';
import { getExceptionListItemSchemaMock } from '../../../common/schemas/response/exception_list_item_schema.mock';
import { HttpStart } from '../../../../../../src/core/public';
import { ApiCallByIdProps } from '../types';
import { ApiCallByIdProps, ApiCallByListIdProps } from '../types';

import { ExceptionsApi, useApi } from './use_api';

Expand Down Expand Up @@ -252,4 +253,116 @@ describe('useApi', () => {
expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});

test('it invokes "fetchExceptionListsItemsByListIds" when "getExceptionItem" used', async () => {
const output = getFoundExceptionListItemSchemaMock();
const onSuccessMock = jest.fn();
const spyOnFetchExceptionListsItemsByListIds = jest
.spyOn(api, 'fetchExceptionListsItemsByListIds')
.mockResolvedValue(output);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

await result.current.getExceptionListsItems({
filterOptions: [],
lists: [{ id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }],
onError: jest.fn(),
onSuccess: onSuccessMock,
pagination: {
page: 1,
perPage: 20,
total: 0,
},
showDetectionsListsOnly: false,
showEndpointListsOnly: false,
});

const expected: ApiCallByListIdProps = {
filterOptions: [],
http: mockKibanaHttpService,
listIds: ['list_id'],
namespaceTypes: ['single'],
pagination: {
page: 1,
perPage: 20,
total: 0,
},
signal: new AbortController().signal,
};

expect(spyOnFetchExceptionListsItemsByListIds).toHaveBeenCalledWith(expected);
expect(onSuccessMock).toHaveBeenCalled();
});
});

test('it does not invoke "fetchExceptionListsItemsByListIds" if no listIds', async () => {
const output = getFoundExceptionListItemSchemaMock();
const onSuccessMock = jest.fn();
const spyOnFetchExceptionListsItemsByListIds = jest
.spyOn(api, 'fetchExceptionListsItemsByListIds')
.mockResolvedValue(output);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

await result.current.getExceptionListsItems({
filterOptions: [],
lists: [{ id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }],
onError: jest.fn(),
onSuccess: onSuccessMock,
pagination: {
page: 1,
perPage: 20,
total: 0,
},
showDetectionsListsOnly: false,
showEndpointListsOnly: true,
});

expect(spyOnFetchExceptionListsItemsByListIds).not.toHaveBeenCalled();
expect(onSuccessMock).toHaveBeenCalledWith({
exceptions: [],
pagination: {
page: 0,
perPage: 20,
total: 0,
},
});
});
});

test('invokes "onError" callback if "fetchExceptionListsItemsByListIds" fails', async () => {
const mockError = new Error('failed to delete item');
jest.spyOn(api, 'fetchExceptionListsItemsByListIds').mockRejectedValue(mockError);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

await result.current.getExceptionListsItems({
filterOptions: [],
lists: [{ id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }],
onError: onErrorMock,
onSuccess: jest.fn(),
pagination: {
page: 1,
perPage: 20,
total: 0,
},
showDetectionsListsOnly: false,
showEndpointListsOnly: false,
});

expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});
});
Loading

0 comments on commit 0756dd3

Please sign in to comment.