[Security Solution][RAC] Flatten alert fields / Require Owner and Space ID fields #107581

madirey · 2021-08-03T19:29:53Z

Summary

This PR flattens the kibana.alert.rule.* fields in the alerts generated from buildAlert. This is for compatibility with the Fields API and general consistency across the RAC implementation.

Additionally, tests have been added for buildAlert and a function getF is introduced to allow us to more easily test alert values across the different implementations while the feature flag is in place for rule_registry.

Finally, we add the kibana.alert.owner and kibana.space_ids fields to the alerts generated from the detection engine.

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

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-08-04T13:29:21Z

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

…-type-flatten

ecezalp

LGTM

...ns/security_solution/server/lib/detection_engine/rule_types/factories/bulk_create_factory.ts

...gins/security_solution/server/lib/detection_engine/rule_types/factories/utils/build_alert.ts

ecezalp · 2021-08-04T14:48:09Z

...gins/security_solution/server/lib/detection_engine/rule_types/factories/utils/build_alert.ts

+export const removeClashes = (doc: SimpleHit) => {
+  if (isWrappedSignalHit(doc)) {
+    invariant(doc._source, '_source field not found');
+    const { signal, ...noSignal } = doc._source;


should we call noSignal source as that's where it seems to be going

...gins/security_solution/server/lib/detection_engine/rule_types/factories/utils/build_alert.ts

.../plugins/security_solution/server/lib/detection_engine/rule_types/factories/utils/flatten.ts

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

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

elasticmachine · 2021-08-04T16:26:05Z

Pinging @elastic/apm-ui (Team:apm)

…-type-flatten

dhurley14

LGTM! Thanks for the additional changes to ensure we are creating alerts with all the required fields for RBAC.

dhurley14 · 2021-08-04T16:36:26Z

...gins/security_solution/server/lib/detection_engine/rule_types/factories/utils/build_alert.ts

-    'kibana.alert.ancestors': ancestors as object[],
+  return ({
+    '@timestamp': new Date().toISOString(),
+    [ALERT_OWNER]: 'siem',


This could be replaced with the const defined here

kibana/x-pack/plugins/security_solution/common/constants.ts

Line 13 in a89f6a1

export const SERVER_APP_ID = 'siem';

madirey · 2021-08-04T20:08:06Z

@elasticmachine merge upstream

…a into security-rule-type-flatten

…-type-flatten

dhurley14 · 2021-08-05T00:38:31Z

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

+};
+
+export const isWrappedSignalHit = (event: SimpleHit): event is WrappedSignalHit => {
+  return (event as WrappedSignalHit)._source.signal != null;


Suggested change

return (event as WrappedSignalHit)._source.signal != null;

return (event as WrappedSignalHit)?._source?.signal != null;

dhurley14 · 2021-08-05T00:38:49Z

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

+};
+
+export const isWrappedRACAlert = (event: SimpleHit): event is WrappedRACAlert => {
+  return (event as WrappedRACAlert)._source['kibana.rac.alert.id'] != null;


Suggested change

return (event as WrappedRACAlert)._source['kibana.rac.alert.id'] != null;

return (event as WrappedRACAlert)?._source?.['kibana.rac.alert.id'] != null;

kibanamachine · 2021-08-05T03:11:32Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	4.3MB	4.3MB	+24.0B
`observability`	507.8KB	508.0KB	+224.0B
`securitySolution`	6.4MB	6.4MB	+200.0B
total			+448.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`apm`	44.4KB	44.6KB	+200.0B
`infra`	149.2KB	149.4KB	+200.0B
`uptime`	34.7KB	34.9KB	+200.0B
total			+600.0B

History

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

* incremental changes * No more type errors * Type guards * Begin adding tests * Flatten * Reduce scope of branch * Remove extraneous argument to filter_duplicate_signals

madirey added 6 commits August 3, 2021 15:24

incremental changes

4f890cb

No more type errors

8ae6b84

Type guards

c871fb2

Begin adding tests

4cf8f7d

Flatten

a25dde0

Reduce scope of branch

8c3a187

madirey requested a review from a team as a code owner August 3, 2021 19:29

madirey added v7.15.0 Feature:RAC label obsolete release_note:skip Skip the PR/issue when compiling release notes labels Aug 3, 2021

Remove extraneous argument to filter_duplicate_signals

b3d42bc

madirey requested review from marshallmain and ecezalp August 4, 2021 13:29

madirey added the Team:Detections and Resp Security Detection Response Team label Aug 4, 2021

madirey enabled auto-merge (squash) August 4, 2021 13:29

madirey added 2 commits August 4, 2021 10:26

Use constants, add producer and space ids

c550247

Merge branch 'master' of github.com:elastic/kibana into security-rule…

79adbfd

…-type-flatten

madirey requested a review from dhurley14 August 4, 2021 14:27

ecezalp approved these changes Aug 4, 2021

View reviewed changes

Make owner and space id required fields

5cccc73

botelastic bot added the Team:APM All issues that need APM UI Team support label Aug 4, 2021

Merge branch 'master' of github.com:elastic/kibana into security-rule…

c7946b9

…-type-flatten

madirey requested a review from a team as a code owner August 4, 2021 16:26

dhurley14 approved these changes Aug 4, 2021

View reviewed changes

madirey added 2 commits August 4, 2021 12:45

Implement suggestions

377ba70

Fix apm tests

75fa25f

madirey changed the title ~~[Security Solution][RAC] Flatten alert fields~~ [Security Solution][RAC] Flatten alert fields / Require Owner and Space ID fields Aug 4, 2021

kibanamachine and others added 4 commits August 4, 2021 16:08

Merge branch 'master' into security-rule-type-flatten

ecece2a

getF -> getField

9e5c00f

Merge branch 'security-rule-type-flatten' of github.com:madirey/kiban…

a58ab61

…a into security-rule-type-flatten

Merge branch 'master' of github.com:elastic/kibana into security-rule…

cd4c45f

…-type-flatten

dhurley14 reviewed Aug 5, 2021

View reviewed changes

Test fixes

da2edb1

peluja1012 requested a review from smith August 5, 2021 02:19

dgieselaar approved these changes Aug 5, 2021

View reviewed changes

madirey merged commit d34cd91 into elastic:master Aug 5, 2021

madirey mentioned this pull request Aug 6, 2021

[7.x] [Security Solution][RAC] Flatten alert fields (#107581) #107832

Merged

dhurley14 mentioned this pull request Aug 9, 2021

[RAC] [RBAC] Adds bulk update route to rule registry and bulk update function to alerts client #106297

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][RAC] Flatten alert fields / Require Owner and Space ID fields #107581

[Security Solution][RAC] Flatten alert fields / Require Owner and Space ID fields #107581

madirey commented Aug 3, 2021 •

edited

Loading

elasticmachine commented Aug 4, 2021

ecezalp left a comment

ecezalp Aug 4, 2021

elasticmachine commented Aug 4, 2021

dhurley14 left a comment

dhurley14 Aug 4, 2021

madirey commented Aug 4, 2021

dhurley14 Aug 5, 2021

dhurley14 Aug 5, 2021

kibanamachine commented Aug 5, 2021

	return (event as WrappedSignalHit)._source.signal != null;
	return (event as WrappedSignalHit)?._source?.signal != null;

	return (event as WrappedRACAlert)._source['kibana.rac.alert.id'] != null;
	return (event as WrappedRACAlert)?._source?.['kibana.rac.alert.id'] != null;

[Security Solution][RAC] Flatten alert fields / Require Owner and Space ID fields #107581

[Security Solution][RAC] Flatten alert fields / Require Owner and Space ID fields #107581

Conversation

madirey commented Aug 3, 2021 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

elasticmachine commented Aug 4, 2021

ecezalp left a comment

Choose a reason for hiding this comment

ecezalp Aug 4, 2021

Choose a reason for hiding this comment

elasticmachine commented Aug 4, 2021

dhurley14 left a comment

Choose a reason for hiding this comment

dhurley14 Aug 4, 2021

Choose a reason for hiding this comment

madirey commented Aug 4, 2021

dhurley14 Aug 5, 2021

Choose a reason for hiding this comment

dhurley14 Aug 5, 2021

Choose a reason for hiding this comment

kibanamachine commented Aug 5, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

madirey commented Aug 3, 2021 •

edited

Loading