Skip to content

Commit

Permalink
[SIEM][Detection Engine] Rule Status Monitoring (elastic#54452)
Browse files Browse the repository at this point in the history
* Working status updates in executor. Need to update read rules api endpoint to only respond with 'status' and not status info. Will create another endpoint to get status details for a rule which will include last five errors (if there are any). Still need tests

* adds new route for getting statuses for a list of given alert ids, adds try-catch and more logic in executor for logging errors, adds scripts and rules for testing, updates find_rules endpoint to display statuses too. Would like to look into using the alerts executor state to better manage logic for statuses, and need to update some types. Also needs unit tests still.

* updated types for routes, updated how merging of alert-to-rule and rule status happens when formatting REST response.

* typecast test server as ServerFacade type

* fix bug where we were not awaiting the accumulated result in the reducer

* update rule status saved object interfaces to play nicely with interfaces provided by saved objects module. Update tests to pass - Need to write new unit tests in an upcoming commit. Next commit will be cleanup from comments then new unit tests.

* fix missed conflicts after rebase

* replace id param with rule.id when searching in statuses, adds sort fields to the saved objects find queries.

* fixes bug where 'executing' statuses were being written into failing historical status list

* camelCase to snake_case in new statuses route, also fix merge conflict

* add deletion of rule statuses to delete_rules_bulk_route. Statuses are created inside of executor so we will not be needing to create statuses directly inside of the create rules bulk route, so I removed that extraneous code.

* pr feedback I forgot to fix earlier

* remove unused import. fixes type check error generated in previous commit

* removes status information from rule when saved to signals index and updates tests to represent this change. Also removes extraneous quotes inserted around alertId field when creating a new historical status.

* adds new bash script to delete all rule statuses, updates error messages in rule statuses to just store actual message, moved querying of rules statuses under a null check, initialize everything to null when first creating rule status, update number of results returned when querying saved objects based on usage, updates saved objects mapping types to use date for dates and keyword for alertId.

* use lodash snake case and update total number of saved objects to return for find rules, delete rules, and read rules.

* updates how statuses are transformed inside of read_rules_route, only update updated_at in rule on update of rule, removes unlabeled todo comment, updates scripts descriptions, removes interval from query_with_rule_id.json sample query, removes debug statement, removes verbose from curl script.

* display rule status on update
  • Loading branch information
dhurley14 committed Jan 14, 2020
1 parent 428fba3 commit bade39e
Show file tree
Hide file tree
Showing 36 changed files with 755 additions and 155 deletions.
1 change: 1 addition & 0 deletions x-pack/legacy/plugins/siem/index.ts
Expand Up @@ -147,6 +147,7 @@ export const siem = (kibana: any) => {
alerting: plugins.alerting,
elasticsearch: plugins.elasticsearch,
spaces: plugins.spaces,
savedObjects: server.savedObjects.SavedObjectsClient,
},
route: route.bind(server),
};
Expand Down
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/siem/server/kibana.index.ts
Expand Up @@ -27,6 +27,7 @@ import { updateRulesBulkRoute } from './lib/detection_engine/routes/rules/update
import { deleteRulesBulkRoute } from './lib/detection_engine/routes/rules/delete_rules_bulk_route';
import { importRulesRoute } from './lib/detection_engine/routes/rules/import_rules_route';
import { exportRulesRoute } from './lib/detection_engine/routes/rules/export_rules_route';
import { findRulesStatusesRoute } from './lib/detection_engine/routes/rules/find_rules_status_route';

const APP_ID = 'siem';

Expand Down Expand Up @@ -54,6 +55,7 @@ export const initServerWithKibana = (context: PluginInitializerContext, __legacy
deleteRulesBulkRoute(__legacy);
importRulesRoute(__legacy);
exportRulesRoute(__legacy);
findRulesStatusesRoute(__legacy);

// Detection Engine Signals routes that have the REST endpoints of /api/detection_engine/signals
// POST /api/detection_engine/signals/status
Expand Down
Expand Up @@ -7,7 +7,7 @@
import Hapi from 'hapi';
import { KibanaConfig } from 'src/legacy/server/kbn_server';
import { ElasticsearchPlugin } from 'src/legacy/core_plugins/elasticsearch';

import { savedObjectsClientMock } from '../../../../../../../../../src/core/server/mocks';
import { alertsClientMock } from '../../../../../../alerting/server/alerts_client.mock';
import { actionsClientMock } from '../../../../../../actions/server/actions_client.mock';

Expand Down Expand Up @@ -48,6 +48,7 @@ export const createMockServer = (config: Record<string, string> = defaultConfig)

const actionsClient = actionsClientMock.create();
const alertsClient = alertsClientMock.create();
const savedObjectsClient = savedObjectsClientMock.create();
const elasticsearch = {
getCluster: jest.fn().mockImplementation(() => ({
callWithRequest: jest.fn(),
Expand All @@ -57,8 +58,15 @@ export const createMockServer = (config: Record<string, string> = defaultConfig)
server.decorate('request', 'getBasePath', () => '/s/default');
server.decorate('request', 'getActionsClient', () => actionsClient);
server.plugins.elasticsearch = (elasticsearch as unknown) as ElasticsearchPlugin;
server.decorate('request', 'getSavedObjectsClient', () => savedObjectsClient);

return { server, alertsClient, actionsClient, elasticsearch };
return {
server,
alertsClient,
actionsClient,
elasticsearch,
savedObjectsClient,
};
};

export const createMockServerWithoutAlertClientDecoration = (
Expand Down
Expand Up @@ -5,6 +5,7 @@
*/

import { ServerInjectOptions } from 'hapi';
import { SavedObjectsFindResponse } from 'kibana/server';
import { ActionResult } from '../../../../../../actions/server/types';
import { SignalsStatusRestParams, SignalsQueryRestParams } from '../../signals/types';
import {
Expand All @@ -15,7 +16,7 @@ import {
INTERNAL_RULE_ID_KEY,
INTERNAL_IMMUTABLE_KEY,
} from '../../../../../common/constants';
import { RuleAlertType } from '../../rules/types';
import { RuleAlertType, IRuleSavedAttributesSavedObjectAttributes } from '../../rules/types';
import { RuleAlertParamsRest } from '../../types';

export const fullRuleAlertParamsRest = (): RuleAlertParamsRest => ({
Expand Down Expand Up @@ -383,3 +384,10 @@ export const getMockPrivileges = () => ({
application: {},
isAuthenticated: false,
});

export const getFindResultStatus = (): SavedObjectsFindResponse<IRuleSavedAttributesSavedObjectAttributes> => ({
page: 1,
per_page: 1,
total: 0,
saved_objects: [],
});
Expand Up @@ -7,6 +7,7 @@
import { createMockServer } from '../__mocks__/_mock_server';
import { getPrivilegeRequest, getMockPrivileges } from '../__mocks__/request_responses';
import { readPrivilegesRoute } from './read_privileges_route';
import { ServerFacade } from '../../../../types';
import * as myUtils from '../utils';

describe('read_privileges', () => {
Expand All @@ -18,7 +19,7 @@ describe('read_privileges', () => {
elasticsearch.getCluster = jest.fn(() => ({
callWithRequest: jest.fn(() => getMockPrivileges()),
}));
readPrivilegesRoute(server);
readPrivilegesRoute((server as unknown) as ServerFacade);
});

afterEach(() => {
Expand Down
Expand Up @@ -12,6 +12,7 @@ import {
} from '../__mocks__/_mock_server';
import { createRulesRoute } from './create_rules_route';
import { ServerInjectOptions } from 'hapi';
import { ServerFacade } from '../../../../types';
import {
getFindResult,
getResult,
Expand All @@ -32,7 +33,7 @@ describe('create_rules_bulk', () => {
callWithRequest: jest.fn().mockImplementation(() => true),
}));

createRulesBulkRoute(server);
createRulesBulkRoute((server as unknown) as ServerFacade);
});

describe('status codes with actionClient and alertClient', () => {
Expand All @@ -47,14 +48,14 @@ describe('create_rules_bulk', () => {

test('returns 404 if actionClient is not available on the route', async () => {
const { serverWithoutActionClient } = createMockServerWithoutActionClientDecoration();
createRulesRoute(serverWithoutActionClient);
createRulesRoute((serverWithoutActionClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutActionClient.inject(getReadBulkRequest());
expect(statusCode).toBe(404);
});

test('returns 404 if alertClient is not available on the route', async () => {
const { serverWithoutAlertClient } = createMockServerWithoutAlertClientDecoration();
createRulesRoute(serverWithoutAlertClient);
createRulesRoute((serverWithoutAlertClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutAlertClient.inject(getReadBulkRequest());
expect(statusCode).toBe(404);
});
Expand All @@ -63,7 +64,7 @@ describe('create_rules_bulk', () => {
const {
serverWithoutActionOrAlertClient,
} = createMockServerWithoutActionOrAlertClientDecoration();
createRulesRoute(serverWithoutActionOrAlertClient);
createRulesRoute((serverWithoutActionOrAlertClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutActionOrAlertClient.inject(getReadBulkRequest());
expect(statusCode).toBe(404);
});
Expand Down
Expand Up @@ -40,8 +40,10 @@ export const createCreateRulesBulkRoute = (server: ServerFacade): Hapi.ServerRou
const actionsClient = isFunction(request.getActionsClient)
? request.getActionsClient()
: null;

if (!alertsClient || !actionsClient) {
const savedObjectsClient = isFunction(request.getSavedObjectsClient)
? request.getSavedObjectsClient()
: null;
if (!alertsClient || !actionsClient || !savedObjectsClient) {
return headers.response().code(404);
}

Expand Down
Expand Up @@ -12,28 +12,43 @@ import {
} from '../__mocks__/_mock_server';
import { createRulesRoute } from './create_rules_route';
import { ServerInjectOptions } from 'hapi';
import { ServerFacade } from '../../../../types';

import {
getFindResult,
getResult,
createActionResult,
getCreateRequest,
typicalPayload,
getFindResultStatus,
} from '../__mocks__/request_responses';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';

describe('create_rules', () => {
let { server, alertsClient, actionsClient, elasticsearch } = createMockServer();
let {
server,
alertsClient,
actionsClient,
elasticsearch,
savedObjectsClient,
} = createMockServer();

beforeEach(() => {
jest.resetAllMocks();
({ server, alertsClient, actionsClient, elasticsearch } = createMockServer());
({
server,
alertsClient,
actionsClient,
elasticsearch,
savedObjectsClient,
} = createMockServer());
elasticsearch.getCluster = jest.fn().mockImplementation(() => ({
callWithRequest: jest
.fn()
.mockImplementation((endpoint, params) => ({ _shards: { total: 1 } })),
}));

createRulesRoute(server);
createRulesRoute((server as unknown) as ServerFacade);
});

describe('status codes with actionClient and alertClient', () => {
Expand All @@ -42,20 +57,21 @@ describe('create_rules', () => {
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
const { statusCode } = await server.inject(getCreateRequest());
expect(statusCode).toBe(200);
});

test('returns 404 if actionClient is not available on the route', async () => {
const { serverWithoutActionClient } = createMockServerWithoutActionClientDecoration();
createRulesRoute(serverWithoutActionClient);
createRulesRoute((serverWithoutActionClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutActionClient.inject(getCreateRequest());
expect(statusCode).toBe(404);
});

test('returns 404 if alertClient is not available on the route', async () => {
const { serverWithoutAlertClient } = createMockServerWithoutAlertClientDecoration();
createRulesRoute(serverWithoutAlertClient);
createRulesRoute((serverWithoutAlertClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutAlertClient.inject(getCreateRequest());
expect(statusCode).toBe(404);
});
Expand All @@ -64,7 +80,7 @@ describe('create_rules', () => {
const {
serverWithoutActionOrAlertClient,
} = createMockServerWithoutActionOrAlertClientDecoration();
createRulesRoute(serverWithoutActionOrAlertClient);
createRulesRoute((serverWithoutActionOrAlertClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutActionOrAlertClient.inject(getCreateRequest());
expect(statusCode).toBe(404);
});
Expand All @@ -76,6 +92,7 @@ describe('create_rules', () => {
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
// missing rule_id should return 200 as it will be auto generated if not given
const { rule_id, ...noRuleId } = typicalPayload();
const request: ServerInjectOptions = {
Expand All @@ -92,6 +109,7 @@ describe('create_rules', () => {
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
const { type, ...noType } = typicalPayload();
const request: ServerInjectOptions = {
method: 'POST',
Expand All @@ -110,6 +128,7 @@ describe('create_rules', () => {
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
const { type, ...noType } = typicalPayload();
const request: ServerInjectOptions = {
method: 'POST',
Expand Down
Expand Up @@ -10,10 +10,11 @@ import Boom from 'boom';
import uuid from 'uuid';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
import { createRules } from '../../rules/create_rules';
import { RulesRequest } from '../../rules/types';
import { RulesRequest, IRuleSavedAttributesSavedObjectAttributes } from '../../rules/types';
import { createRulesSchema } from '../schemas/create_rules_schema';
import { ServerFacade } from '../../../../types';
import { readRules } from '../../rules/read_rules';
import { ruleStatusSavedObjectType } from '../../rules/saved_object_mappings';
import { transformOrError } from './utils';
import { getIndexExists } from '../../index/get_index_exists';
import { callWithRequestFactory, getIndex, transformError } from '../utils';
Expand Down Expand Up @@ -65,8 +66,10 @@ export const createCreateRulesRoute = (server: ServerFacade): Hapi.ServerRoute =
const actionsClient = isFunction(request.getActionsClient)
? request.getActionsClient()
: null;

if (!alertsClient || !actionsClient) {
const savedObjectsClient = isFunction(request.getSavedObjectsClient)
? request.getSavedObjectsClient()
: null;
if (!alertsClient || !actionsClient || !savedObjectsClient) {
return headers.response().code(404);
}

Expand Down Expand Up @@ -120,7 +123,17 @@ export const createCreateRulesRoute = (server: ServerFacade): Hapi.ServerRoute =
references,
version: 1,
});
return transformOrError(createdRule);
const ruleStatuses = await savedObjectsClient.find<
IRuleSavedAttributesSavedObjectAttributes
>({
type: ruleStatusSavedObjectType,
perPage: 1,
sortField: 'statusDate',
sortOrder: 'desc',
search: `${createdRule.id}`,
searchFields: ['alertId'],
});
return transformOrError(createdRule, ruleStatuses.saved_objects[0]);
} catch (err) {
return transformError(err);
}
Expand Down
Expand Up @@ -20,18 +20,20 @@ import {
getDeleteBulkRequestById,
getDeleteAsPostBulkRequest,
getDeleteAsPostBulkRequestById,
getFindResultStatus,
} from '../__mocks__/request_responses';
import { ServerFacade } from '../../../../types';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';

import { deleteRulesBulkRoute } from './delete_rules_bulk_route';
import { BulkError } from '../utils';

describe('delete_rules', () => {
let { server, alertsClient } = createMockServer();
let { server, alertsClient, savedObjectsClient } = createMockServer();

beforeEach(() => {
({ server, alertsClient } = createMockServer());
deleteRulesBulkRoute(server);
({ server, alertsClient, savedObjectsClient } = createMockServer());
deleteRulesBulkRoute((server as unknown) as ServerFacade);
});

afterEach(() => {
Expand Down Expand Up @@ -83,6 +85,8 @@ describe('delete_rules', () => {
alertsClient.find.mockResolvedValue(getFindResult());
alertsClient.get.mockResolvedValue(getResult());
alertsClient.delete.mockResolvedValue({});
savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
savedObjectsClient.delete.mockResolvedValue({});
const { payload } = await server.inject(getDeleteBulkRequest());
const parsed: BulkError[] = JSON.parse(payload);
const expected: BulkError[] = [
Expand All @@ -96,14 +100,14 @@ describe('delete_rules', () => {

test('returns 404 if actionClient is not available on the route', async () => {
const { serverWithoutActionClient } = createMockServerWithoutActionClientDecoration();
deleteRulesBulkRoute(serverWithoutActionClient);
deleteRulesBulkRoute((serverWithoutActionClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutActionClient.inject(getDeleteBulkRequest());
expect(statusCode).toBe(404);
});

test('returns 404 if alertClient is not available on the route', async () => {
const { serverWithoutAlertClient } = createMockServerWithoutAlertClientDecoration();
deleteRulesBulkRoute(serverWithoutAlertClient);
deleteRulesBulkRoute((serverWithoutAlertClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutAlertClient.inject(getDeleteBulkRequest());
expect(statusCode).toBe(404);
});
Expand All @@ -112,7 +116,7 @@ describe('delete_rules', () => {
const {
serverWithoutActionOrAlertClient,
} = createMockServerWithoutActionOrAlertClientDecoration();
deleteRulesBulkRoute(serverWithoutActionOrAlertClient);
deleteRulesBulkRoute((serverWithoutActionOrAlertClient as unknown) as ServerFacade);
const { statusCode } = await serverWithoutActionOrAlertClient.inject(getDeleteBulkRequest());
expect(statusCode).toBe(404);
});
Expand Down

0 comments on commit bade39e

Please sign in to comment.