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

dhurley14 · 2021-02-01T20:33:53Z

Summary

Removes allow_no_indices param. Specifying this option meant that our field_capabilities check was throwing an error if the rule included an index pattern that did not exist. This would be most typical in e.g. a prepackaged rule using the default index patterns.

If we pass the field capabilities api an index pattern that does not match any indices in Elasticsearch, that index pattern is not returned as a part of the response with indices that have that field "unmapped". This adds a check if the response from field capabilities api has empty indices property then write error status with index patterns provided to rule. This essentially tells the user there are no concrete indices matching any of the provided index patterns.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-02-01T21:10:29Z

Pinging @elastic/security-solution (Team: SecuritySolution)

elasticmachine · 2021-02-01T21:10:29Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts

rylnd

It looks like there are still some failing integration tests that were relying on the previous rule behavior. Related to this, it would be nice to have an integration test that explicitly captures this new behavior: the rule will fail with an error if no source indices exist.

LGTM once CI is happy 👍

dhurley14 · 2021-02-02T13:45:14Z

@elasticmachine merge upstream

…ices property then write error status with index patterns provided to rule

…e should fail and when one index pattern does exist but another does not, the rule should succeed

kibanamachine · 2021-02-02T18:16:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e41de31

Metrics [docs]

✅ unchanged

History

💚 Build #102985 succeeded c60f8e9
💚 Build #102862 succeeded a387f20
💔 Build #102812 failed a820420
💔 Build #102775 failed 4733f5e
💔 Build #102751 failed 14ef150

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…rror being thrown in response of field capabilities (elastic#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

…-ml-jobs * 'master' of github.com:elastic/kibana: (254 commits) [Security Solution] [Detections] Remove allow_no_indices to prevent error being thrown in response of field capabilities (elastic#89927) Skip test for cloud (elastic#89450) [Fleet] Fix duplicate data streams being shown in UI (elastic#89812) Bump package dependencies (elastic#90034) [App Search] DRY helper for encoding/decoding routes that can have special characters in params (elastic#89811) TypeScript project references for Observability plugin (elastic#89320) [SearchSource] Combine sort and parent fields when serializing (elastic#89808) Made imports static (elastic#89935) [ml] migrate file_data_visualizer/import route to file_upload plugin (elastic#89640) [Discover] Adapt default column behavior (elastic#89826) Round start and end values (elastic#89030) Rename getProxyAgents to getCustomAgents (elastic#89813) [Form lib] UseField `onError` listener (elastic#89895) [APM] use latency sum instead of avg for impact (elastic#89990) migrate more core-owned plugins to tsproject ref (elastic#89975) [Logs UI] Load <LogStream> entries via async searches (elastic#86899) [APM] Abort browser requests when appropriate (elastic#89557) [Alerting] Allow user to select existing connector of same type when fixing broken connector (elastic#89062) [Data Table] Use shared CSV export mechanism (elastic#89702) chore(NA): improve logic check when installing Bazel tools (elastic#89634) ...

…vent error being thrown in response of field capabilities (#89927) (#90072) * 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

…event error being thrown in response of field capabilities (#89927) (#90073) * 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

rylnd · 2021-02-04T21:15:01Z

x-pack/test/detection_engine_api_integration/basic/tests/create_rules.ts

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


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

dhurley14 requested review from a team as code owners February 1, 2021 20:33

dhurley14 self-assigned this Feb 1, 2021

dhurley14 added the Feature:Detection Rules Anything related to Security Solution's Detection Rules label Feb 1, 2021

dhurley14 changed the title ~~[Security Solution] [Detections]~~ [Security Solution] [Detections] Remove allow_no_indices to prevent error being thrown in response of field capabilities Feb 1, 2021

marshallmain reviewed Feb 1, 2021

View reviewed changes

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts Outdated Show resolved Hide resolved

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts Outdated Show resolved Hide resolved

rylnd approved these changes Feb 2, 2021

View reviewed changes

marshallmain approved these changes Feb 2, 2021

View reviewed changes

dhurley14 added 5 commits February 2, 2021 10:39

remove allow_no_indices param, adds a check if response has empty ind…

425dd32

…ices property then write error status with index patterns provided to rule

fix tests

80a296b

fix tests and update with comments

6433552

update integration tests

76017b8

adds integration test for when an index pattern doesn't exist the rul…

e41de31

…e should fail and when one index pattern does exist but another does not, the rule should succeed

dhurley14 force-pushed the fix-privileges-indices-error branch from c60f8e9 to e41de31 Compare February 2, 2021 16:34

dhurley14 merged commit 198c6fb into elastic:master Feb 2, 2021

dhurley14 deleted the fix-privileges-indices-error branch February 2, 2021 19:07

dhurley14 mentioned this pull request Feb 2, 2021

[7.x] [Security Solution] [Detections] Remove allow_no_indices to prevent error being thrown in response of field capabilities (#89927) #90072

Merged

dhurley14 mentioned this pull request Feb 2, 2021

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

Merged

rylnd reviewed Feb 4, 2021

View reviewed changes

rylnd mentioned this pull request Feb 10, 2021

[Security Solution][Detections] Prevent permissions checks from being bypassed when indices do not exist #89664

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dhurley14 commented Feb 1, 2021 •

edited

Loading

elasticmachine commented Feb 1, 2021

elasticmachine commented Feb 1, 2021

rylnd left a comment

dhurley14 commented Feb 2, 2021

kibanamachine commented Feb 2, 2021

rylnd Feb 4, 2021

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

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

Conversation

dhurley14 commented Feb 1, 2021 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 1, 2021

elasticmachine commented Feb 1, 2021

rylnd left a comment

Choose a reason for hiding this comment

dhurley14 commented Feb 2, 2021

kibanamachine commented Feb 2, 2021

💚 Build Succeeded

Metrics [docs]

History

rylnd Feb 4, 2021

Choose a reason for hiding this comment

dhurley14 commented Feb 1, 2021 •

edited

Loading