Skip to content

Commit

Permalink
[SIEM][Detection Engine] Fixes queries to ignore errors when signals …
Browse files Browse the repository at this point in the history
…index is not present (elastic#57750)

## Summary

Fixes a bug where if you try to query the signals index and it is not present you would get an error that would bubble up to the UI.

Before:
<img width="824" alt="Screen Shot 2020-02-13 at 12 57 55 PM" src="https://user-images.githubusercontent.com/1151048/74499587-74881d00-4ea1-11ea-93b0-8d7258f79404.png">

After:
<img width="790" alt="Screen Shot 2020-02-13 at 8 43 28 PM" src="https://user-images.githubusercontent.com/1151048/74499619-9386af00-4ea1-11ea-889b-cb1853e399e3.png">

<img width="804" alt="Screen Shot 2020-02-13 at 8 43 34 PM" src="https://user-images.githubusercontent.com/1151048/74499624-971a3600-4ea1-11ea-862b-9695ecbadcbe.png">


Also fixes a regression bug with error toasters showing up when they shouldn't because of a signals index not existing. This only effects 7.6.x and master and is _not_ part of 7.6.0 at the moment.

* elastic#57641

### 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://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~~

~~- [ ] [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)~~

~~- [ ] 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://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

### 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)~~
  • Loading branch information
FrankHassanabad authored Feb 15, 2020
1 parent 3bc0dfa commit 420e92e
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import { MessageBody } from '../../../../components/ml/api/throw_if_not_ok';

export class SignalIndexError extends Error {
message: string = '';
statusCode: number = -1;
status_code: number = -1;
error: string = '';

constructor(errObj: MessageBody) {
super(errObj.message);
this.message = errObj.message ?? '';
this.statusCode = errObj.statusCode ?? -1;
this.status_code = errObj.status_code ?? -1;
this.error = errObj.error ?? '';
this.name = 'SignalIndexError';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const useSignalIndex = (): Return => {
signalIndexName: null,
createDeSignalIndex: createIndex,
});
if (error instanceof SignalIndexError && error.statusCode !== 404) {
if (error instanceof SignalIndexError && error.status_code !== 404) {
errorToToaster({ title: i18n.SIGNAL_GET_NAME_FAILURE, error, dispatchToaster });
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const setSignalsStatusRouteDef = (
},
query: queryObject,
},
ignoreUnavailable: true,
});
} catch (exc) {
// error while getting or updating signal with id: id in signal index .siem-signals
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const querySignalsRouteDef = (
return clusterClient.callAsCurrentUser('search', {
index,
body: { query, aggs, _source, track_total_hits, size },
ignoreUnavailable: true,
});
} catch (exc) {
// error while getting or updating signal with id: id in signal index .siem-signals
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ export default ({ loadTestFile }: FtrProviderContext): void => {
loadTestFile(require.resolve('./update_rules_bulk'));
loadTestFile(require.resolve('./patch_rules_bulk'));
loadTestFile(require.resolve('./patch_rules'));
loadTestFile(require.resolve('./query_signals'));
loadTestFile(require.resolve('./open_close_signals'));
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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 expect from '@kbn/expect';

import { DETECTION_ENGINE_SIGNALS_STATUS_URL } from '../../../../legacy/plugins/siem/common/constants';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import {
createSignalsIndex,
deleteSignalsIndex,
setSignalStatus,
getSignalStatusEmptyResponse,
} from './utils';

// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext) => {
const supertest = getService('supertest');

describe('open_close_signals', () => {
describe('validation checks', () => {
it('should not give errors when querying and the signals index does not exist yet', async () => {
const { body } = await supertest
.post(DETECTION_ENGINE_SIGNALS_STATUS_URL)
.set('kbn-xsrf', 'true')
.send(setSignalStatus({ signalIds: ['123'], status: 'open' }))
.expect(200);

// remove any server generated items that are indeterministic
delete body.took;

expect(body).to.eql(getSignalStatusEmptyResponse());
});

it('should not give errors when querying and the signals index does exist and is empty', async () => {
await createSignalsIndex(supertest);
const { body } = await supertest
.post(DETECTION_ENGINE_SIGNALS_STATUS_URL)
.set('kbn-xsrf', 'true')
.send(setSignalStatus({ signalIds: ['123'], status: 'open' }))
.expect(200);

// remove any server generated items that are indeterministic
delete body.took;

expect(body).to.eql(getSignalStatusEmptyResponse());

await deleteSignalsIndex(supertest);
});
});
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* 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 expect from '@kbn/expect';

import { DETECTION_ENGINE_QUERY_SIGNALS_URL } from '../../../../legacy/plugins/siem/common/constants';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import { getSignalStatus, createSignalsIndex, deleteSignalsIndex } from './utils';

// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext) => {
const supertest = getService('supertest');

describe('query_signals_route', () => {
describe('validation checks', () => {
it('should not give errors when querying and the signals index does not exist yet', async () => {
const { body } = await supertest
.post(DETECTION_ENGINE_QUERY_SIGNALS_URL)
.set('kbn-xsrf', 'true')
.send(getSignalStatus())
.expect(200);

// remove any server generated items that are indeterministic
delete body.took;

expect(body).to.eql({
timed_out: false,
_shards: { total: 0, successful: 0, skipped: 0, failed: 0 },
hits: { total: { value: 0, relation: 'eq' }, max_score: 0, hits: [] },
});
});

it('should not give errors when querying and the signals index does exist and is empty', async () => {
await createSignalsIndex(supertest);
const { body } = await supertest
.post(DETECTION_ENGINE_QUERY_SIGNALS_URL)
.set('kbn-xsrf', 'true')
.send(getSignalStatus())
.expect(200);

// remove any server generated items that are indeterministic
delete body.took;

expect(body).to.eql({
timed_out: false,
_shards: { total: 1, successful: 1, skipped: 0, failed: 0 },
hits: { total: { value: 0, relation: 'eq' }, max_score: null, hits: [] },
aggregations: {
statuses: { doc_count_error_upper_bound: 0, sum_other_doc_count: 0, buckets: [] },
},
});

await deleteSignalsIndex(supertest);
});
});
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,36 @@ export const getSimpleRule = (ruleId = 'rule-1'): Partial<OutputRuleAlertRest> =
query: 'user.name: root or user.name: admin',
});

export const getSignalStatus = () => ({
aggs: { statuses: { terms: { field: 'signal.status', size: 10 } } },
});

export const setSignalStatus = ({
signalIds,
status,
}: {
signalIds: string[];
status: 'open' | 'closed';
}) => ({
signal_ids: signalIds,
status,
});

export const getSignalStatusEmptyResponse = () => ({
timed_out: false,
total: 0,
updated: 0,
deleted: 0,
batches: 0,
version_conflicts: 0,
noops: 0,
retries: { bulk: 0, search: 0 },
throttled_millis: 0,
requests_per_second: -1,
throttled_until_millis: 0,
failures: [],
});

/**
* This is a typical simple rule for testing that is easy for most basic testing
*/
Expand Down

0 comments on commit 420e92e

Please sign in to comment.