Skip to content

Commit

Permalink
[SIEM][Detection Engine] Backend end-to-end tests (#57385)
Browse files Browse the repository at this point in the history
## Summary

* Adds end to end integration tests
* Fixes a bug with import where on imports it was forcing all rules that were being imported to be set to be "enabled: false" instead of honoring what the original export has set for its enabled.
* Adds a few "to be safe" await block so that the front end does not get a race condition within the bulk deletes and other parts of the code.
* Fixes `statusCode` to be `status_code` and removes most of the Hapi Boomer errors
* Changes PUT to be PATCH for partial updates
* Adds true updates with PUT
* Put some TODO blocks around existing bugs found in the API in the e2e tests that we might have time to get to or might not. This will let others maintaining the tests know that once they fix the bug they should update the end to end test to change the behavior.  

Testing this:

Go to the latest CI logs and look for any particular lines from the test executing such as:

```ts
should set the response content types to be expected
```

Also run this manually on your machine through this command:

```ts
node scripts/functional_tests --config x-pack/test/detection_engine_api_integration/security_and_spaces/config.ts
```

Change a test manually and re-run the above command to watch something fail.


Screen shot of what you should see on the CI machine when these are running:
<img width="1825" alt="Screen Shot 2020-02-08 at 10 15 21 AM" src="https://user-images.githubusercontent.com/1151048/74089355-ae9a8e80-4a5d-11ea-9050-86e68d7e3bba.png">


### Checklist

Delete any items that are not applicable to this PR.

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

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

- [x] [Unit or functional tests](https:/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)~~

~~- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)~~

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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled appropriately](https:/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
  • Loading branch information
FrankHassanabad authored Feb 12, 2020
1 parent f0cffca commit 7339d02
Show file tree
Hide file tree
Showing 98 changed files with 6,537 additions and 667 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const enableRules = async ({ ids, enabled }: EnableRulesProps): Promise<R
const response = await KibanaServices.get().http.fetch<Rule[]>(
`${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
{
method: 'PUT',
method: 'PATCH',
body: JSON.stringify(ids.map(id => ({ id, enabled }))),
asResponse: true,
}
Expand All @@ -160,7 +160,7 @@ export const deleteRules = async ({ ids }: DeleteRulesProps): Promise<Array<Rule
const response = await KibanaServices.get().http.fetch<Rule[]>(
`${DETECTION_ENGINE_RULES_URL}/_bulk_delete`,
{
method: 'PUT',
method: 'DELETE',
body: JSON.stringify(ids.map(id => ({ id }))),
asResponse: true,
}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/siem/public/hooks/api/api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export const throwIfNotOk = async (response?: Response): Promise<void> => {
if (body != null && body.message) {
if (body.statusCode != null) {
throw new ToasterErrors([body.message, `${i18n.STATUS_CODE} ${body.statusCode}`]);
} else if (body.status_code != null) {
throw new ToasterErrors([body.message, `${i18n.STATUS_CODE} ${body.status_code}`]);
} else {
throw new ToasterErrors([body.message]);
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/legacy/plugins/siem/public/utils/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface MessageBody {
error?: string;
message?: string;
statusCode?: number;
status_code?: number;
}

export const parseJsonFromBody = async (response: Response): Promise<MessageBody | null> => {
Expand Down
8 changes: 6 additions & 2 deletions x-pack/legacy/plugins/siem/server/kibana.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { readIndexRoute } from './lib/detection_engine/routes/index/read_index_r
import { readRulesRoute } from './lib/detection_engine/routes/rules/read_rules_route';
import { findRulesRoute } from './lib/detection_engine/routes/rules/find_rules_route';
import { deleteRulesRoute } from './lib/detection_engine/routes/rules/delete_rules_route';
import { updateRulesRoute } from './lib/detection_engine/routes/rules/update_rules_route';
import { patchRulesRoute } from './lib/detection_engine/routes/rules/patch_rules_route';
import { setSignalsStatusRoute } from './lib/detection_engine/routes/signals/open_close_signals_route';
import { querySignalsRoute } from './lib/detection_engine/routes/signals/query_signals_route';
import { ServerFacade } from './types';
Expand All @@ -23,12 +23,14 @@ import { readTagsRoute } from './lib/detection_engine/routes/tags/read_tags_rout
import { readPrivilegesRoute } from './lib/detection_engine/routes/privileges/read_privileges_route';
import { addPrepackedRulesRoute } from './lib/detection_engine/routes/rules/add_prepackaged_rules_route';
import { createRulesBulkRoute } from './lib/detection_engine/routes/rules/create_rules_bulk_route';
import { updateRulesBulkRoute } from './lib/detection_engine/routes/rules/update_rules_bulk_route';
import { patchRulesBulkRoute } from './lib/detection_engine/routes/rules/patch_rules_bulk_route';
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';
import { getPrepackagedRulesStatusRoute } from './lib/detection_engine/routes/rules/get_prepackaged_rules_status_route';
import { updateRulesRoute } from './lib/detection_engine/routes/rules/update_rules_route';
import { updateRulesBulkRoute } from './lib/detection_engine/routes/rules/update_rules_bulk_route';

const APP_ID = 'siem';

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

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

importRulesRoute(__legacy);
exportRulesRoute(__legacy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ export const getUpdateRequest = (): ServerInjectOptions => ({
},
});

export const getPatchRequest = (): ServerInjectOptions => ({
method: 'PATCH',
url: DETECTION_ENGINE_RULES_URL,
payload: {
...typicalPayload(),
},
});

export const getReadRequest = (): ServerInjectOptions => ({
method: 'GET',
url: `${DETECTION_ENGINE_RULES_URL}?rule_id=rule-1`,
Expand All @@ -130,6 +138,12 @@ export const getUpdateBulkRequest = (): ServerInjectOptions => ({
payload: [typicalPayload()],
});

export const getPatchBulkRequest = (): ServerInjectOptions => ({
method: 'PATCH',
url: `${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
payload: [typicalPayload()],
});

export const getDeleteBulkRequest = (): ServerInjectOptions => ({
method: 'DELETE',
url: `${DETECTION_ENGINE_RULES_URL}/_bulk_delete`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import Hapi from 'hapi';
import Boom from 'boom';

import { DETECTION_ENGINE_INDEX_URL } from '../../../../../common/constants';
import signalsPolicy from './signals_policy.json';
Expand All @@ -31,13 +30,18 @@ export const createCreateIndexRoute = (server: ServerFacade): Hapi.ServerRoute =
},
},
},
async handler(request: RequestFacade) {
async handler(request: RequestFacade, headers) {
try {
const index = getIndex(request, server);
const callWithRequest = callWithRequestFactory(request, server);
const indexExists = await getIndexExists(callWithRequest, index);
if (indexExists) {
return new Boom(`index: "${index}" already exists`, { statusCode: 409 });
return headers
.response({
message: `index: "${index}" already exists`,
status_code: 409,
})
.code(409);
} else {
const policyExists = await getPolicyExists(callWithRequest, index);
if (!policyExists) {
Expand All @@ -52,7 +56,13 @@ export const createCreateIndexRoute = (server: ServerFacade): Hapi.ServerRoute =
return { acknowledged: true };
}
} catch (err) {
return transformError(err);
const error = transformError(err);
return headers
.response({
message: error.message,
status_code: error.statusCode,
})
.code(error.statusCode);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import Hapi from 'hapi';
import Boom from 'boom';

import { DETECTION_ENGINE_INDEX_URL } from '../../../../../common/constants';
import { ServerFacade, RequestFacade } from '../../../../types';
Expand Down Expand Up @@ -39,13 +38,18 @@ export const createDeleteIndexRoute = (server: ServerFacade): Hapi.ServerRoute =
},
},
},
async handler(request: RequestFacade) {
async handler(request: RequestFacade, headers) {
try {
const index = getIndex(request, server);
const callWithRequest = callWithRequestFactory(request, server);
const indexExists = await getIndexExists(callWithRequest, index);
if (!indexExists) {
return new Boom(`index: "${index}" does not exist`, { statusCode: 404 });
return headers
.response({
message: `index: "${index}" does not exist`,
status_code: 404,
})
.code(404);
} else {
await deleteAllIndex(callWithRequest, `${index}-*`);
const policyExists = await getPolicyExists(callWithRequest, index);
Expand All @@ -59,7 +63,13 @@ export const createDeleteIndexRoute = (server: ServerFacade): Hapi.ServerRoute =
return { acknowledged: true };
}
} catch (err) {
return transformError(err);
const error = transformError(err);
return headers
.response({
message: error.message,
status_code: error.statusCode,
})
.code(error.statusCode);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import Hapi from 'hapi';
import Boom from 'boom';

import { DETECTION_ENGINE_INDEX_URL } from '../../../../../common/constants';
import { ServerFacade, RequestFacade } from '../../../../types';
Expand Down Expand Up @@ -42,11 +41,22 @@ export const createReadIndexRoute = (server: ServerFacade): Hapi.ServerRoute =>
if (request.method.toLowerCase() === 'head') {
return headers.response().code(404);
} else {
return new Boom('index for this space does not exist', { statusCode: 404 });
return headers
.response({
message: 'index for this space does not exist',
status_code: 404,
})
.code(404);
}
}
} catch (err) {
return transformError(err);
const error = transformError(err);
return headers
.response({
message: error.message,
status_code: error.statusCode,
})
.code(error.statusCode);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const createReadPrivilegesRulesRoute = (server: ServerFacade): Hapi.Serve
},
},
},
async handler(request: RulesRequest) {
async handler(request: RulesRequest, headers) {
try {
const callWithRequest = callWithRequestFactory(request, server);
const index = getIndex(request, server);
Expand All @@ -35,7 +35,13 @@ export const createReadPrivilegesRulesRoute = (server: ServerFacade): Hapi.Serve
has_encryption_key: !usingEphemeralEncryptionKey,
});
} catch (err) {
return transformError(err);
const error = transformError(err);
return headers
.response({
message: error.message,
status_code: error.statusCode,
})
.code(error.statusCode);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,9 @@ describe('add_prepackaged_rules_route', () => {
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,
status_code: 400,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import Hapi from 'hapi';
import { isFunction } from 'lodash/fp';
import Boom from 'boom';

import { DETECTION_ENGINE_PREPACKAGED_URL } from '../../../../../common/constants';
import { ServerFacade, RequestFacade } from '../../../../types';
Expand Down Expand Up @@ -56,9 +55,12 @@ export const createAddPrepackedRulesRoute = (server: ServerFacade): Hapi.ServerR
if (rulesToInstall.length !== 0 || rulesToUpdate.length !== 0) {
const spaceIndexExists = await getIndexExists(callWithRequest, spaceIndex);
if (!spaceIndexExists) {
return Boom.badRequest(
`Pre-packaged rules cannot be installed until the space index is created: ${spaceIndex}`
);
return headers
.response({
message: `Pre-packaged rules cannot be installed until the space index is created: ${spaceIndex}`,
status_code: 400,
})
.code(400);
}
}
await Promise.all(
Expand All @@ -76,7 +78,13 @@ export const createAddPrepackedRulesRoute = (server: ServerFacade): Hapi.ServerR
rules_updated: rulesToUpdate.length,
};
} catch (err) {
return transformError(err);
const error = transformError(err);
return headers
.response({
message: error.message,
status_code: error.statusCode,
})
.code(error.statusCode);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,8 @@ describe('create_rules', () => {
alertsClient.create.mockResolvedValue(getResult());
const { payload } = await server.inject(getCreateRequest());
expect(JSON.parse(payload)).toEqual({
error: 'Bad Request',
message: 'To create a rule, the index must exist first. Index .siem-signals does not exist',
statusCode: 400,
status_code: 400,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import Hapi from 'hapi';
import { isFunction } from 'lodash/fp';
import Boom from 'boom';
import uuid from 'uuid';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
import { createRules } from '../../rules/create_rules';
Expand All @@ -15,7 +14,7 @@ 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 { transform } from './utils';
import { getIndexExists } from '../../index/get_index_exists';
import { callWithRequestFactory, getIndex, transformError } from '../utils';
import { KibanaRequest } from '../../../../../../../../../src/core/server';
Expand Down Expand Up @@ -76,14 +75,22 @@ export const createCreateRulesRoute = (server: ServerFacade): Hapi.ServerRoute =
const callWithRequest = callWithRequestFactory(request, server);
const indexExists = await getIndexExists(callWithRequest, finalIndex);
if (!indexExists) {
return Boom.badRequest(
`To create a rule, the index must exist first. Index ${finalIndex} does not exist`
);
return headers
.response({
message: `To create a rule, the index must exist first. Index ${finalIndex} does not exist`,
status_code: 400,
})
.code(400);
}
if (ruleId != null) {
const rule = await readRules({ alertsClient, ruleId });
if (rule != null) {
return Boom.conflict(`rule_id: "${ruleId}" already exists`);
return headers
.response({
message: `rule_id: "${ruleId}" already exists`,
status_code: 409,
})
.code(409);
}
}
const createdRule = await createRules({
Expand Down Expand Up @@ -126,9 +133,25 @@ export const createCreateRulesRoute = (server: ServerFacade): Hapi.ServerRoute =
search: `${createdRule.id}`,
searchFields: ['alertId'],
});
return transformOrError(createdRule, ruleStatuses.saved_objects[0]);
const transformed = transform(createdRule, ruleStatuses.saved_objects[0]);
if (transformed == null) {
return headers
.response({
message: 'Internal error transforming rules',
status_code: 500,
})
.code(500);
} else {
return transformed;
}
} catch (err) {
return transformError(err);
const error = transformError(err);
return headers
.response({
message: error.message,
status_code: error.statusCode,
})
.code(error.statusCode);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const createDeleteRulesBulkRoute = (server: ServerFacade): Hapi.ServerRou
if (!alertsClient || !savedObjectsClient) {
return headers.response().code(404);
}
const rules = Promise.all(
const rules = await Promise.all(
request.payload.map(async payloadRule => {
const { id, rule_id: ruleId } = payloadRule;
const idOrRuleIdOrUnknown = id ?? ruleId ?? '(unknown id)';
Expand Down
Loading

0 comments on commit 7339d02

Please sign in to comment.