Skip to content

Commit

Permalink
[SIEM][Detection Engine] Adds critical missing status route to prepac…
Browse files Browse the repository at this point in the history
…kaged rules

## Summary

* Fixes a critical bug where the missing status for the REST route was missing
* Fixes a bug with the 400 not being used for the missing index in some cases
* Changes create and update to NO LONGER allow immutable to be passed
* Fixes a bug with the add prepackaged schema to where you could use `immutable: false`. Now it is required to be missing or set to `immutable: true` within it. 
* Cleans up unit tests

To use the critical bug missing status for the REST route:

```ts
GET /api/detection_engine/rules/prepackaged/_status 
```

And you will get back:

```ts
{
  "rules_installed": 252,
  "rules_not_installed": 87,
  "rules_not_updated": 0
}
```

See the script:

```ts
get_prepackaged_rules_status.sh
```

for more details

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
  • Loading branch information
FrankHassanabad authored and jkelastic committed Jan 17, 2020
1 parent f536f5a commit 10fdbd1
Show file tree
Hide file tree
Showing 38 changed files with 788 additions and 312 deletions.
5 changes: 5 additions & 0 deletions x-pack/legacy/plugins/siem/server/kibana.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { deleteRulesBulkRoute } from './lib/detection_engine/routes/rules/delete
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';
import { getPrepackagedRulesStatusRoute } from './lib/detection_engine/routes/rules/get_prepackaged_rules_status_route';

const APP_ID = 'siem';

Expand All @@ -49,12 +50,16 @@ export const initServerWithKibana = (context: PluginInitializerContext, __legacy
updateRulesRoute(__legacy);
deleteRulesRoute(__legacy);
findRulesRoute(__legacy);

addPrepackedRulesRoute(__legacy);
getPrepackagedRulesStatusRoute(__legacy);
createRulesBulkRoute(__legacy);
updateRulesBulkRoute(__legacy);
deleteRulesBulkRoute(__legacy);

importRulesRoute(__legacy);
exportRulesRoute(__legacy);

findRulesStatusesRoute(__legacy);

// Detection Engine Signals routes that have the REST endpoints of /api/detection_engine/signals
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ 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';
import { APP_ID, SIGNALS_INDEX_KEY } from '../../../../../common/constants';
import { ServerFacade } from '../../../../types';

const defaultConfig = {
'kibana.index': '.kibana',
[`xpack.${APP_ID}.${SIGNALS_INDEX_KEY}`]: '.siem-signals',
};

const isKibanaConfig = (config: unknown): config is KibanaConfig =>
Expand Down Expand Up @@ -58,10 +61,10 @@ 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.plugins.spaces = { getSpaceId: () => 'default' };
server.decorate('request', 'getSavedObjectsClient', () => savedObjectsClient);

return {
server,
server: server as ServerFacade & Hapi.Server,
alertsClient,
actionsClient,
elasticsearch,
Expand All @@ -82,7 +85,10 @@ export const createMockServerWithoutAlertClientDecoration = (
serverWithoutAlertClient.decorate('request', 'getBasePath', () => '/s/default');
serverWithoutAlertClient.decorate('request', 'getActionsClient', () => actionsClient);

return { serverWithoutAlertClient, actionsClient };
return {
serverWithoutAlertClient: serverWithoutAlertClient as ServerFacade & Hapi.Server,
actionsClient,
};
};

export const createMockServerWithoutActionClientDecoration = (
Expand All @@ -98,7 +104,10 @@ export const createMockServerWithoutActionClientDecoration = (
serverWithoutActionClient.decorate('request', 'getBasePath', () => '/s/default');
serverWithoutActionClient.decorate('request', 'getAlertsClient', () => alertsClient);

return { serverWithoutActionClient, alertsClient };
return {
serverWithoutActionClient: serverWithoutActionClient as ServerFacade & Hapi.Server,
alertsClient,
};
};

export const createMockServerWithoutActionOrAlertClientDecoration = (
Expand All @@ -111,6 +120,22 @@ export const createMockServerWithoutActionOrAlertClientDecoration = (
serverWithoutActionOrAlertClient.config = () => createMockKibanaConfig(config);

return {
serverWithoutActionOrAlertClient,
serverWithoutActionOrAlertClient: serverWithoutActionOrAlertClient as ServerFacade &
Hapi.Server,
};
};

export const getMockIndexName = () =>
jest.fn().mockImplementation(() => ({
callWithRequest: jest.fn().mockImplementationOnce(() => 'index-name'),
}));

export const getMockEmptyIndex = () =>
jest.fn().mockImplementation(() => ({
callWithRequest: jest.fn().mockImplementation(() => ({ _shards: { total: 0 } })),
}));

export const getMockNonEmptyIndex = () =>
jest.fn().mockImplementation(() => ({
callWithRequest: jest.fn().mockImplementation(() => ({ _shards: { total: 1 } })),
}));
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
DETECTION_ENGINE_QUERY_SIGNALS_URL,
INTERNAL_RULE_ID_KEY,
INTERNAL_IMMUTABLE_KEY,
DETECTION_ENGINE_PREPACKAGED_URL,
} from '../../../../../common/constants';
import { RuleAlertType, IRuleSavedAttributesSavedObjectAttributes } from '../../rules/types';
import { RuleAlertParamsRest } from '../../types';
Expand Down Expand Up @@ -157,7 +158,17 @@ export const getDeleteAsPostBulkRequest = (): ServerInjectOptions => ({

export const getPrivilegeRequest = (): ServerInjectOptions => ({
method: 'GET',
url: `${DETECTION_ENGINE_PRIVILEGES_URL}`,
url: DETECTION_ENGINE_PRIVILEGES_URL,
});

export const addPrepackagedRulesRequest = (): ServerInjectOptions => ({
method: 'PUT',
url: DETECTION_ENGINE_PREPACKAGED_URL,
});

export const getPrepackagedRulesStatusRequest = (): ServerInjectOptions => ({
method: 'GET',
url: `${DETECTION_ENGINE_PREPACKAGED_URL}/_status`,
});

export interface FindHit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
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 @@ -19,7 +18,7 @@ describe('read_privileges', () => {
elasticsearch.getCluster = jest.fn(() => ({
callWithRequest: jest.fn(() => getMockPrivileges()),
}));
readPrivilegesRoute((server as unknown) as ServerFacade);
readPrivilegesRoute(server);
});

afterEach(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import {
createMockServer,
createMockServerWithoutActionClientDecoration,
createMockServerWithoutAlertClientDecoration,
createMockServerWithoutActionOrAlertClientDecoration,
getMockEmptyIndex,
getMockNonEmptyIndex,
} from '../__mocks__/_mock_server';
import { createRulesRoute } from './create_rules_route';
import {
getFindResult,
getResult,
createActionResult,
addPrepackagedRulesRequest,
getFindResultWithSingleHit,
} from '../__mocks__/request_responses';

jest.mock('../../rules/get_prepackaged_rules', () => {
return {
getPrepackagedRules: () => {
return [
{
rule_id: 'rule-1',
output_index: '.siem-signals',
risk_score: 50,
description: 'some description',
from: 'now-5m',
to: 'now',
index: ['index-1'],
name: 'some-name',
severity: 'severity',
interval: '5m',
type: 'query',
version: 2, // set one higher than the mocks which is set to 1 to trigger updates
},
];
},
};
});

import { addPrepackedRulesRoute } from './add_prepackaged_rules_route';

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

beforeEach(() => {
jest.resetAllMocks();
({ server, alertsClient, actionsClient, elasticsearch } = createMockServer());
elasticsearch.getCluster = getMockNonEmptyIndex();

addPrepackedRulesRoute(server);
});

describe('status codes with actionClient and alertClient', () => {
test('returns 200 when creating a with a valid actionClient and alertClient', async () => {
alertsClient.find.mockResolvedValue(getFindResult());
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
const { statusCode } = await server.inject(addPrepackagedRulesRequest());
expect(statusCode).toBe(200);
});

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

test('returns 404 if alertClient is not available on the route', async () => {
const { serverWithoutAlertClient } = createMockServerWithoutAlertClientDecoration();
createRulesRoute(serverWithoutAlertClient);
const { statusCode } = await serverWithoutAlertClient.inject(addPrepackagedRulesRequest());
expect(statusCode).toBe(404);
});

test('returns 404 if alertClient and actionClient are both not available on the route', async () => {
const {
serverWithoutActionOrAlertClient,
} = createMockServerWithoutActionOrAlertClientDecoration();
createRulesRoute(serverWithoutActionOrAlertClient);
const { statusCode } = await serverWithoutActionOrAlertClient.inject(
addPrepackagedRulesRequest()
);
expect(statusCode).toBe(404);
});
});

describe('validation', () => {
test('it returns a 400 if the index does not exist', async () => {
elasticsearch.getCluster = getMockEmptyIndex();
alertsClient.find.mockResolvedValue(getFindResult());
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
const { payload } = await server.inject(addPrepackagedRulesRequest());
expect(JSON.parse(payload)).toEqual({
error: 'Bad Request',
message:
'Pre-packaged rules cannot be installed until the space index is created: .siem-signals-default',
statusCode: 400,
});
});
});

describe('payload', () => {
test('1 rule is installed and 0 are updated when find results are empty', async () => {
alertsClient.find.mockResolvedValue(getFindResult());
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
const { payload } = await server.inject(addPrepackagedRulesRequest());
expect(JSON.parse(payload)).toEqual({
rules_installed: 1,
rules_updated: 0,
});
});

test('1 rule is updated and 0 are installed when we return a single find and the versions are different', async () => {
alertsClient.find.mockResolvedValue(getFindResultWithSingleHit());
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
const { payload } = await server.inject(addPrepackagedRulesRequest());
expect(JSON.parse(payload)).toEqual({
rules_installed: 0,
rules_updated: 1,
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ export const createAddPrepackedRulesRoute = (server: ServerFacade): Hapi.ServerR
const callWithRequest = callWithRequestFactory(request, server);
const rulesFromFileSystem = getPrepackagedRules();

const prepackedRules = await getExistingPrepackagedRules({ alertsClient });
const rulesToInstall = getRulesToInstall(rulesFromFileSystem, prepackedRules);
const rulesToUpdate = getRulesToUpdate(rulesFromFileSystem, prepackedRules);
const prepackagedRules = await getExistingPrepackagedRules({ alertsClient });
const rulesToInstall = getRulesToInstall(rulesFromFileSystem, prepackagedRules);
const rulesToUpdate = getRulesToUpdate(rulesFromFileSystem, prepackagedRules);

const spaceIndex = getIndex(request, server);
if (rulesToInstall.length !== 0 || rulesToUpdate.length !== 0) {
const spaceIndexExists = await getIndexExists(callWithRequest, spaceIndex);
if (!spaceIndexExists) {
throw new Boom(
return Boom.badRequest(
`Pre-packaged rules cannot be installed until the space index is created: ${spaceIndex}`
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import {
createMockServerWithoutActionClientDecoration,
createMockServerWithoutAlertClientDecoration,
createMockServerWithoutActionOrAlertClientDecoration,
getMockEmptyIndex,
} from '../__mocks__/_mock_server';
import { createRulesRoute } from './create_rules_route';
import { ServerInjectOptions } from 'hapi';
import { ServerFacade } from '../../../../types';
import {
getFindResult,
getResult,
Expand All @@ -29,11 +29,7 @@ describe('create_rules_bulk', () => {
beforeEach(() => {
jest.resetAllMocks();
({ server, alertsClient, actionsClient, elasticsearch } = createMockServer());
elasticsearch.getCluster = jest.fn().mockImplementation(() => ({
callWithRequest: jest.fn().mockImplementation(() => true),
}));

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

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

test('returns 404 if actionClient is not available on the route', async () => {
const { serverWithoutActionClient } = createMockServerWithoutActionClientDecoration();
createRulesRoute((serverWithoutActionClient as unknown) as ServerFacade);
createRulesRoute(serverWithoutActionClient);
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 as unknown) as ServerFacade);
createRulesRoute(serverWithoutAlertClient);
const { statusCode } = await serverWithoutAlertClient.inject(getReadBulkRequest());
expect(statusCode).toBe(404);
});
Expand All @@ -64,13 +60,32 @@ describe('create_rules_bulk', () => {
const {
serverWithoutActionOrAlertClient,
} = createMockServerWithoutActionOrAlertClientDecoration();
createRulesRoute((serverWithoutActionOrAlertClient as unknown) as ServerFacade);
createRulesRoute(serverWithoutActionOrAlertClient);
const { statusCode } = await serverWithoutActionOrAlertClient.inject(getReadBulkRequest());
expect(statusCode).toBe(404);
});
});

describe('validation', () => {
test('it gets a 409 if the index does not exist', async () => {
elasticsearch.getCluster = getMockEmptyIndex();
alertsClient.find.mockResolvedValue(getFindResult());
alertsClient.get.mockResolvedValue(getResult());
actionsClient.create.mockResolvedValue(createActionResult());
alertsClient.create.mockResolvedValue(getResult());
const { payload } = await server.inject(getReadBulkRequest());
expect(JSON.parse(payload)).toEqual([
{
error: {
message:
'To create a rule, the index must exist first. Index .siem-signals does not exist',
status_code: 400,
},
rule_id: 'rule-1',
},
]);
});

test('returns 200 if rule_id is not given as the id is auto generated from the alert framework', async () => {
alertsClient.find.mockResolvedValue(getFindResult());
alertsClient.get.mockResolvedValue(getResult());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export const createCreateRulesBulkRoute = (server: ServerFacade): Hapi.ServerRou
if (!indexExists) {
return createBulkErrorObject({
ruleId: ruleIdOrUuid,
statusCode: 409,
statusCode: 400,
message: `To create a rule, the index must exist first. Index ${finalIndex} does not exist`,
});
}
Expand Down
Loading

0 comments on commit 10fdbd1

Please sign in to comment.