Skip to content

Commit

Permalink
[Security Solution] [Detections] Remove allow_no_indices to prevent e…
Browse files Browse the repository at this point in the history 
…rror being thrown in response of field capabilities (#89927)

* remove allow_no_indices param, adds a check if response has empty indices property then write error status with index patterns provided to rule

* fix tests

* fix tests and update with comments

* update integration tests

* adds integration test for when an index pattern doesn't exist the rule should fail and when one index pattern does exist but another does not, the rule should succeed
  • Loading branch information
@dhurley14
dhurley14 authored Feb 2, 2021
1 parent be41a9f commit 198c6fb
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 12 deletions.
1 change: 1 addition & 0 deletions ...gins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ describe('rules_notification_alert_type', () => {
const value: Partial<ApiResponse> = {
statusCode: 200,
body: {
indices: ['index1', 'index2', 'index3', 'index4'],
fields: {
'@timestamp': {
date: {
Expand Down
15 changes: 4 additions & 11 deletions ...k/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { Logger, KibanaRequest } from 'src/core/server';
import isEmpty from 'lodash/isEmpty';
import { chain, tryCatch } from 'fp-ts/lib/TaskEither';
import { flow, pipe } from 'fp-ts/lib/function';
import { flow } from 'fp-ts/lib/function';

import { toError, toPromise } from '../../../../common/fp_utils';

Expand Down Expand Up @@ -188,22 +188,14 @@ export const signalRulesAlertType = ({
try {
if (!isEmpty(index)) {
const hasTimestampOverride = timestampOverride != null && !isEmpty(timestampOverride);
const inputIndices = await getInputIndex(services, version, index);
const [privileges, timestampFieldCaps] = await Promise.all([
pipe(
{ services, version, index },
({ services: svc, version: ver, index: idx }) =>
pipe(
tryCatch(() => getInputIndex(svc, ver, idx), toError),
chain((indices) => tryCatch(() => checkPrivileges(svc, indices), toError))
),
toPromise
),
checkPrivileges(services, inputIndices),
services.scopedClusterClient.fieldCaps({
index,
fields: hasTimestampOverride
? ['@timestamp', timestampOverride as string]
: ['@timestamp'],
allow_no_indices: false,
include_unmapped: true,
}),
]);
Expand All @@ -222,6 +214,7 @@ export const signalRulesAlertType = ({
wroteStatus,
hasTimestampOverride ? (timestampOverride as string) : '@timestamp',
timestampFieldCaps,
inputIndices,
ruleStatusService,
logger,
buildRuleMessage
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,7 @@ describe('utils', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const timestampFieldCapsResponse: Partial<ApiResponse<Record<string, any>, Context>> = {
body: {
indices: ['myfakeindex-1', 'myfakeindex-2', 'myfakeindex-3', 'myfakeindex-4'],
fields: {
[timestampField]: {
date: {
Expand All @@ -843,6 +844,7 @@ describe('utils', () => {
timestampField,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
timestampFieldCapsResponse as ApiResponse<Record<string, any>>,
['myfa*'],
ruleStatusServiceMock,
mockLogger,
buildRuleMessage
Expand All @@ -857,6 +859,7 @@ describe('utils', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const timestampFieldCapsResponse: Partial<ApiResponse<Record<string, any>, Context>> = {
body: {
indices: ['myfakeindex-1', 'myfakeindex-2', 'myfakeindex-3', 'myfakeindex-4'],
fields: {
[timestampField]: {
date: {
Expand All @@ -881,6 +884,7 @@ describe('utils', () => {
timestampField,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
timestampFieldCapsResponse as ApiResponse<Record<string, any>>,
['myfa*'],
ruleStatusServiceMock,
mockLogger,
buildRuleMessage
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,19 @@ export const hasTimestampFields = async (
// node_modules/@elastic/elasticsearch/api/kibana.d.ts
// eslint-disable-next-line @typescript-eslint/no-explicit-any
timestampFieldCapsResponse: ApiResponse<Record<string, any>, Context>,
inputIndices: string[],
ruleStatusService: RuleStatusService,
logger: Logger,
buildRuleMessage: BuildRuleMessage
): Promise<boolean> => {
if (
if (!wroteStatus && isEmpty(timestampFieldCapsResponse.body.indices)) {
const errorString = `The following index patterns did not match any indices: ${JSON.stringify(
inputIndices
)}`;
logger.error(buildRuleMessage(errorString));
await ruleStatusService.error(errorString);
return true;
} else if (
!wroteStatus &&
(isEmpty(timestampFieldCapsResponse.body.fields) ||
timestampFieldCapsResponse.body.fields[timestampField] == null ||
Expand Down
3 changes: 3 additions & 0 deletions x-pack/test/detection_engine_api_integration/basic/tests/create_rules.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext) => {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');

describe('create_rules', () => {
describe('validation errors', () => {
Expand All @@ -46,11 +47,13 @@ export default ({ getService }: FtrProviderContext) => {
describe('creating rules', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await esArchiver.unload('auditbeat/hosts');
});

it('should create a single rule with a rule_id', async () => {
Expand Down
3 changes: 3 additions & 0 deletions x-pack/test/detection_engine_api_integration/basic/tests/create_rules_bulk.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');

describe('create_rules_bulk', () => {
describe('validation errors', () => {
Expand All @@ -49,11 +50,13 @@ export default ({ getService }: FtrProviderContext): void => {
describe('creating rules in bulk', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await esArchiver.unload('auditbeat/hosts');
});

it('should create a single rule with a rule_id', async () => {
Expand Down
3 changes: 3 additions & 0 deletions x-pack/test/detection_engine_api_integration/basic/tests/find_statuses.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,20 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');
const es = getService('es');

describe('find_statuses', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await deleteAllRulesStatuses(es);
await esArchiver.unload('auditbeat/hosts');
});

it('should return an empty find statuses body correctly if no statuses are loaded', async () => {
Expand Down
3 changes: 3 additions & 0 deletions x-pack/test/detection_engine_api_integration/security_and_spaces/tests/add_actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,19 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext) => {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');

describe('add_actions', () => {
describe('adding actions', () => {
beforeEach(async () => {
await esArchiver.load('auditbeat/hosts');
await createSignalsIndex(supertest);
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await esArchiver.unload('auditbeat/hosts');
});

it('should be able to create a new webhook action and attach it to a rule', async () => {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_exceptions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ export default ({ getService }: FtrProviderContext) => {
describe('creating rules with exceptions', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await deleteAllExceptions(es);
await esArchiver.unload('auditbeat/hosts');
});

it('should create a single rule with a rule_id and add an exception list to the rule', async () => {
Expand Down
43 changes: 43 additions & 0 deletions x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,13 @@ export default ({ getService }: FtrProviderContext) => {
describe('creating rules', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await esArchiver.unload('auditbeat/hosts');
});

it('should create a single rule with a rule_id', async () => {
Expand Down Expand Up @@ -111,6 +113,47 @@ export default ({ getService }: FtrProviderContext) => {
expect(statusBody[body.id].current_status.status).to.eql('succeeded');
});

it('should create a single rule with a rule_id and an index pattern that does not match anything available and fail the rule', async () => {
const simpleRule = getRuleForSignalTesting(['does-not-exist-*']);
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(simpleRule)
.expect(200);

await waitForRuleSuccessOrStatus(supertest, body.id, 'failed');

const { body: statusBody } = await supertest
.post(DETECTION_ENGINE_RULES_STATUS_URL)
.set('kbn-xsrf', 'true')
.send({ ids: [body.id] })
.expect(200);

expect(statusBody[body.id].current_status.status).to.eql('failed');
expect(statusBody[body.id].current_status.last_failure_message).to.eql(
'The following index patterns did not match any indices: ["does-not-exist-*"]'
);
});

it('should create a single rule with a rule_id and an index pattern that does not match anything and an index pattern that does and the rule should be successful', async () => {
const simpleRule = getRuleForSignalTesting(['does-not-exist-*', 'auditbeat-*']);
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(simpleRule)
.expect(200);

await waitForRuleSuccessOrStatus(supertest, body.id, 'succeeded');

const { body: statusBody } = await supertest
.post(DETECTION_ENGINE_RULES_STATUS_URL)
.set('kbn-xsrf', 'true')
.send({ ids: [body.id] })
.expect(200);

expect(statusBody[body.id].current_status.status).to.eql('succeeded');
});

it('should create a single rule without an input index', async () => {
const rule: CreateRulesSchema = {
name: 'Simple Rule Query',
Expand Down
3 changes: 3 additions & 0 deletions x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');

describe('create_rules_bulk', () => {
describe('validation errors', () => {
Expand All @@ -54,11 +55,13 @@ export default ({ getService }: FtrProviderContext): void => {
describe('creating rules in bulk', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await esArchiver.unload('auditbeat/hosts');
});

it('should create a single rule with a rule_id', async () => {
Expand Down
3 changes: 3 additions & 0 deletions x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,19 @@ import {
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const es = getService('es');
const esArchiver = getService('esArchiver');

describe('find_statuses', () => {
beforeEach(async () => {
await createSignalsIndex(supertest);
await esArchiver.load('auditbeat/hosts');
});

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(supertest);
await deleteAllRulesStatuses(es);
await esArchiver.unload('auditbeat/hosts');
});

it('should return an empty find statuses body correctly if no statuses are loaded', async () => {
Expand Down

0 comments on commit 198c6fb

Please sign in to comment.