Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] [Detections] Remove allow_no_indices to prevent error being thrown in response of field capabilities #89927

Merged
merged 5 commits into from
Feb 2, 2021
Merged

[Security Solution] [Detections] Remove allow_no_indices to prevent error being thrown in response of field capabilities #89927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
425dd32
remove allow_no_indices param, adds a check if response has empty ind…
dhurley14 Feb 1, 2021
80a296b
fix tests
dhurley14 Feb 1, 2021
6433552
fix tests and update with comments
dhurley14 Feb 1, 2021
76017b8
update integration tests
dhurley14 Feb 2, 2021
e41de31
adds integration test for when an index pattern doesn't exist the rul…
dhurley14 Feb 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...gins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: these indices were added so that the rule would not fail on the initial permissions check due to a lack of indices.

});

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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