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][Endpoint] Update list api summary endpoint to use filter #123476

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
* Side Public License, v 1.
*/

import { ID, LIST_ID, NAMESPACE_TYPE } from '../../constants/index.mock';
import { FILTER, ID, LIST_ID, NAMESPACE_TYPE } from '../../constants/index.mock';

import { SummaryExceptionListSchema } from '.';

export const getSummaryExceptionListSchemaMock = (): SummaryExceptionListSchema => ({
filter: FILTER,
id: ID,
list_id: LIST_ID,
namespace_type: NAMESPACE_TYPE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ describe('summary_exception_list_schema', () => {
expect(message.schema).toEqual(payload);
});

test('it should accept an undefined for "filter"', () => {
const payload = getSummaryExceptionListSchemaMock();
delete payload.filter;
const decoded = summaryExceptionListSchema.decode(payload);
const checked = exactCheck(payload, decoded);
const message = foldLeftRight(checked);

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

test('it should accept an undefined for "id"', () => {
const payload = getSummaryExceptionListSchemaMock();
delete payload.id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import * as t from 'io-ts';
import { NamespaceType } from '../../common/default_namespace';
import { RequiredKeepUndefined } from '../../common/required_keep_undefined';
import { id } from '../../common/id';
import { filter, Filter } from '../../common/filter';
import { list_id } from '../../common/list_id';
import { namespace_type } from '../../common/namespace_type';

export const summaryExceptionListSchema = t.exact(
t.partial({
filter,
id,
list_id,
namespace_type, // defaults to 'single' if not set during decode
Expand All @@ -30,4 +32,5 @@ export type SummaryExceptionListSchemaDecoded = Omit<
'namespace_type'
> & {
namespace_type: NamespaceType;
filter: Filter;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { ExceptionListSummarySchema } from '@kbn/securitysolution-io-ts-list-types';

export const getSummaryExceptionListSchemaMock = (
overrides?: Partial<ExceptionListSummarySchema>
): ExceptionListSummarySchema => {
return {
linux: 0,
macos: 0,
total: 0,
windows: 0,
...(overrides || {}),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ export const summaryExceptionListRoute = (router: ListsPluginRouter): void => {
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);
try {
const { id, list_id: listId, namespace_type: namespaceType } = request.query;
const { id, list_id: listId, namespace_type: namespaceType, filter } = request.query;
const exceptionLists = getExceptionListClient(context);
if (id != null || listId != null) {
const exceptionListSummary = await exceptionLists.getExceptionListSummary({
filter,
id,
listId,
namespaceType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,20 @@ export class ExceptionListClient {

/**
* Fetch an exception list parent container
* @params filter {sting | undefined} kql "filter" expression
* @params listId {string | undefined} the "list_id" of an exception list
* @params id {string | undefined} the "id" of an exception list
* @params namespaceType {string | undefined} saved object namespace (single | agnostic)
* @return {ExceptionListSummarySchema | null} summary of exception list item os types
*/
public getExceptionListSummary = async ({
filter,
listId,
id,
namespaceType,
}: GetExceptionListSummaryOptions): Promise<ExceptionListSummarySchema | null> => {
const { savedObjectsClient } = this;
return getExceptionListSummary({ id, listId, namespaceType, savedObjectsClient });
return getExceptionListSummary({ filter, id, listId, namespaceType, savedObjectsClient });
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export interface GetExceptionListOptions {
}

export interface GetExceptionListSummaryOptions {
filter: FilterOrUndefined;
listId: ListIdOrUndefined;
id: IdOrUndefined;
namespaceType: NamespaceType;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { ExceptionListSummarySchema } from '@kbn/securitysolution-io-ts-list-types';

import type { SavedObjectsClientContract } from '../../../../../../src/core/server';
import { savedObjectsClientMock } from '../../../../../../src/core/server/mocks';

import { getExceptionListSummary } from './get_exception_list_summary';

describe('get_exception_list_summary', () => {
describe('getExceptionListSummary', () => {
let savedObjectsClient: jest.Mocked<SavedObjectsClientContract>;

beforeEach(() => {
savedObjectsClient = savedObjectsClientMock.create();
});

test('it should aggregate items if not host isolation exception artifact', async () => {
const savedObject = {
aggregations: {
by_os: {
buckets: [
{ doc_count: 2, key: 'linux' },
{ doc_count: 3, key: 'macos' },
{ doc_count: 5, key: 'windows' },
],
doc_count_error_upper_bound: 0,
sum_other_doc_count: 0,
},
},
page: 1,
per_page: 0,
saved_objects: [],
total: 7,
};
savedObjectsClient.find.mockResolvedValue(savedObject);

const summary = (await getExceptionListSummary({
filter: undefined,
id: undefined,
listId: '',
namespaceType: 'agnostic',
savedObjectsClient,
})) as ExceptionListSummarySchema;

expect(summary.total).toEqual(10);
});

test('it should NOT aggregate items if host isolation exception artifact', async () => {
const savedObject = {
aggregations: {
by_os: {
buckets: [
{ doc_count: 3, key: 'linux' },
{ doc_count: 3, key: 'macos' },
{ doc_count: 3, key: 'windows' },
],
doc_count_error_upper_bound: 0,
sum_other_doc_count: 0,
},
},
page: 1,
per_page: 0,
saved_objects: [],
total: 7,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not relevant but I think this total number should be 6 according the counters above

Copy link
Member Author

Choose a reason for hiding this comment

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

@dasansol92 so this was because the API was returning +1 as total. I went looking for why that was and @paul-tavares helped me figure that out. We made the changes in bdc0c97 to fix that. Also updated the test mock to have the correct total.

};
savedObjectsClient.find.mockResolvedValue(savedObject);

const summary = (await getExceptionListSummary({
filter: undefined,
id: undefined,
listId: 'endpoint_host_isolation_exceptions',
namespaceType: 'agnostic',
savedObjectsClient,
})) as ExceptionListSummarySchema;

expect(summary.total).toEqual(3);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type {
ExceptionListSummarySchema,
FilterOrUndefined,
IdOrUndefined,
ListIdOrUndefined,
NamespaceType,
Expand All @@ -20,6 +21,7 @@ import {
import { ExceptionListSoSchema } from '../../schemas/saved_objects';

interface GetExceptionListSummaryOptions {
filter: FilterOrUndefined;
id: IdOrUndefined;
listId: ListIdOrUndefined;
savedObjectsClient: SavedObjectsClientContract;
Expand All @@ -37,6 +39,7 @@ interface ByOsAggType {
}

export const getExceptionListSummary = async ({
filter,
id,
listId,
savedObjectsClient,
Expand Down Expand Up @@ -67,7 +70,7 @@ export const getExceptionListSummary = async ({
},
},
},
filter: `${savedObjectType}.attributes.list_type: item`,
filter,
perPage: 0,
search: finalListId,
searchFields: ['list_id'],
Expand All @@ -84,7 +87,12 @@ export const getExceptionListSummary = async ({
(acc, item: ByOsAggBucketType) => ({
...acc,
[item.key]: item.doc_count,
total: acc.total + item.doc_count,
total:
// Do not add up the items by OS if host isolation exception
// As each host exception entry applies to all OSs
Copy link
Member

Choose a reason for hiding this comment

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

thanks for commenting the "why" here 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

😄

listId === 'endpoint_host_isolation_exceptions'
ashokaditya marked this conversation as resolved.
Show resolved Hide resolved
? item.doc_count
: acc.total + item.doc_count,
}),
{ linux: 0, macos: 0, total: 0, windows: 0 }
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class EventFiltersHttpService implements EventFiltersService {
return deleteOne(this.http, id);
}

async getSummary(): Promise<ExceptionListSummarySchema> {
return getSummary(this.http);
async getSummary(filter?: string): Promise<ExceptionListSummarySchema> {
return getSummary({ http: this.http, filter });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,17 @@ export async function deleteOne(http: HttpStart, id: string): Promise<ExceptionL
});
}

export async function getSummary(http: HttpStart): Promise<ExceptionListSummarySchema> {
export async function getSummary({
http,
filter,
}: {
http: HttpStart;
filter?: string;
}): Promise<ExceptionListSummarySchema> {
await ensureEventFiltersListExists(http);
return http.get<ExceptionListSummarySchema>(`${EXCEPTION_LIST_URL}/summary`, {
query: {
filter,
list_id: ENDPOINT_EVENT_FILTERS_LIST_ID,
namespace_type: 'agnostic',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { combineReducers, createStore } from 'redux';
import type {
FoundExceptionListItemSchema,
ExceptionListItemSchema,
ExceptionListSummarySchema,
} from '@kbn/securitysolution-io-ts-list-types';
import { EXCEPTION_LIST_ITEM_URL, EXCEPTION_LIST_URL } from '@kbn/securitysolution-list-constants';
import { Ecs } from '../../../../../common/ecs';
Expand All @@ -25,6 +26,7 @@ import {
} from '../../../../common/mock/endpoint/http_handler_mock_factory';
import { getFoundExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/found_exception_list_item_schema.mock';
import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock';
import { getSummaryExceptionListSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_summary_schema.mock';

export const createGlobalNoMiddlewareStore = () => {
return createStore(
Expand Down Expand Up @@ -107,6 +109,7 @@ export type EventFiltersListQueryHttpMockProviders = ResponseProvidersInterface<
eventFiltersGetOne: () => ExceptionListItemSchema;
eventFiltersCreateOne: () => ExceptionListItemSchema;
eventFiltersUpdateOne: () => ExceptionListItemSchema;
eventFiltersGetSummary: () => ExceptionListSummarySchema;
}>;

export const esResponseData = () => ({
Expand Down Expand Up @@ -255,4 +258,12 @@ export const eventFiltersListQueryHttpMock =
return getExceptionListItemSchemaMock();
},
},
{
id: 'eventFiltersGetSummary',
method: 'get',
path: `${EXCEPTION_LIST_URL}/summary`,
handler: (): ExceptionListSummarySchema => {
return getSummaryExceptionListSchemaMock();
},
},
]);
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface EventFiltersService {
getOne(id: string): Promise<ExceptionListItemSchema>;
updateOne(exception: Immutable<UpdateExceptionListItemSchema>): Promise<ExceptionListItemSchema>;
deleteOne(id: string): Promise<ExceptionListItemSchema>;
getSummary(): Promise<ExceptionListSummarySchema>;
getSummary(filter?: string): Promise<ExceptionListSummarySchema>;
}

export interface EventFiltersListPageData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { FleetIntegrationEventFiltersCard } from './fleet_integration_event_filt
import { EndpointDocGenerator } from '../../../../../../../../common/endpoint/generate_data';
import { getPolicyEventFiltersPath } from '../../../../../../common/routing';
import { PolicyData } from '../../../../../../../../common/endpoint/types';
import { getFoundExceptionListItemSchemaMock } from '../../../../../../../../../lists/common/schemas/response/found_exception_list_item_schema.mock';
import { getSummaryExceptionListSchemaMock } from '../../../../../../../../../lists/common/schemas/response/exception_list_summary_schema.mock';

const endpointGenerator = new EndpointDocGenerator('seed');

Expand All @@ -40,7 +40,9 @@ describe('Fleet integration policy endpoint security event filters card', () =>
renderResult = mockedContext.render(
<FleetIntegrationEventFiltersCard policyId={policy.id} />
);
await waitFor(() => expect(mockedApi.responseProvider.eventFiltersList).toHaveBeenCalled());
await waitFor(() =>
expect(mockedApi.responseProvider.eventFiltersGetSummary).toHaveBeenCalled()
);
});
return renderResult;
};
Expand All @@ -51,8 +53,8 @@ describe('Fleet integration policy endpoint security event filters card', () =>
afterEach(() => reactTestingLibrary.cleanup());

it('should call the API and render the card correctly', async () => {
mockedApi.responseProvider.eventFiltersList.mockReturnValue(
getFoundExceptionListItemSchemaMock(3)
mockedApi.responseProvider.eventFiltersGetSummary.mockReturnValue(
getSummaryExceptionListSchemaMock({ total: 3 })
);

await render();
Expand All @@ -62,17 +64,17 @@ describe('Fleet integration policy endpoint security event filters card', () =>
});

it('should show the card even when no event filters associated with the policy', async () => {
mockedApi.responseProvider.eventFiltersList.mockReturnValue(
getFoundExceptionListItemSchemaMock(0)
mockedApi.responseProvider.eventFiltersGetSummary.mockReturnValue(
getSummaryExceptionListSchemaMock({ total: 0 })
);

await render();
expect(renderResult.getByTestId('eventFilters-fleet-integration-card')).toBeTruthy();
});

it('should have the correct manage event filters link', async () => {
mockedApi.responseProvider.eventFiltersList.mockReturnValue(
getFoundExceptionListItemSchemaMock(1)
mockedApi.responseProvider.eventFiltersGetSummary.mockReturnValue(
getSummaryExceptionListSchemaMock({ total: 1 })
);

await render();
Expand All @@ -84,7 +86,7 @@ describe('Fleet integration policy endpoint security event filters card', () =>

it('should show an error toast when API request fails', async () => {
const error = new Error('Uh oh! API error!');
mockedApi.responseProvider.eventFiltersList.mockImplementation(() => {
mockedApi.responseProvider.eventFiltersGetSummary.mockImplementation(() => {
throw error;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,7 @@ export const FleetIntegrationEventFiltersCard = memo<{
isMounted.current = true;
const fetchStats = async () => {
try {
const summary = await eventFiltersApi.getList({
perPage: 1,
page: 1,
filter: parsePoliciesToKQL([policyId, 'all']),
});
const summary = await eventFiltersApi.getSummary(parsePoliciesToKQL([policyId, 'all']));
if (isMounted.current) {
setStats({
total: summary.total,
ashokaditya marked this conversation as resolved.
Show resolved Hide resolved
Expand Down