[RAC] [RBAC] add space ids array to each alert document #105173

dhurley14 · 2021-07-10T23:56:20Z

Summary

Since we cannot provide the space id during the generation of the security URI's in the feature privilege builder step (in plugin setup phase) we must filter them out during the fetch of the alert.

Checklist

Delete any items that are not applicable to this PR.

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

x-pack/test/apm_api_integration/configs/index.ts

packages/kbn-rule-data-utils/src/technical_field_names.ts

… of this could go in the authorization class but I don't think we have access to the spaceids when we generate the kibana security action strings?

…pdates jest tests, adds integration tests

…e_ids to kibana.space_ids

…na.space_ids in rule registry integration tests and apm integration tests

xcrzx

Thanks for the update, LGTM 👍

xcrzx · 2021-07-16T09:55:41Z

x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts

+    // not sure why typescript needs the non-null assertion here
+    // we already assert the value is not undefined with the ternary
+    // still getting an error with the ternary.. strange.


This is a Typescript limitation. It is not smart enough to narrow down the type of a computed property inside if-else:

const obj: { prop?: string } = {} if (obj.prop) { obj.prop.toLowerCase() // <- guard works, no type error } const prop: 'prop' = 'prop' if (obj[prop]) { obj[prop].toLowerCase() // <- guard doesn't work, object is possibly 'undefined' error }

x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md

x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts

ymao1

Alerting changes LGTM

ogupte

APM test updates LGTM

kibanamachine · 2021-07-16T20:05:44Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`alerting`	228	229	+1

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	+350.0B
`observability`	482.7KB	483.1KB	+405.0B
total			+755.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`	40.3KB	40.3KB	+55.0B
`infra`	147.2KB	147.3KB	+55.0B
total			+110.0B

Unknown metric groups

API count

id	before	after	diff
`alerting`	236	237	+1

History

💚 Build #139155 succeeded 48e1b91
💚 Build #138963 succeeded bd6c938
💔 Build #138938 failed 93b858c
💔 Build #138899 failed 896d304
💔 Build #138877 failed 58be86e

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

cc @dhurley14

kibanamachine · 2021-07-16T20:08:17Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 105173

marshallmain · 2021-07-16T20:18:30Z

x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts

+    event[SPACE_IDS] =
+      event[SPACE_IDS] == null
+        ? [spaceId]
+        : [spaceId, ...event[SPACE_IDS]!.filter((sid) => sid !== spaceId)];


It looks like we're only adding space IDs for lifecycle alerts. Is there something preventing us from injecting the space ID inside the RuleDataClient bulk method so that persistence rule types and any future rule types will also automatically inject the space ID?

This is true we could push down the space id from the plugins into the rule data client, which would then be accessible via the bulk method. I'll take a look at this.

We would end up iterating over the data passed into bulk twice rather than once (like in the executor definitions), so not that optimal.

* kind of working solution... need to fix types.. would be great if all of this could go in the authorization class but I don't think we have access to the spaceids when we generate the kibana security action strings? * update mapping type as array:true for space_ids field, fixes types, updates jest tests, adds integration tests * undo changes in alerting authz class * update snapshot for apm api integration test for rules writing alerts * fix apm integration tests * omit version and sequence from expected outcome * re-add space id after this code was moved in master * add another default space id to test * fixes bug to remove duplicate spaceids * add space ids filter to elasticsearch query, updates detection role * update snapshot * update type docs for alerts client * remove dead code * fix type error * renames space ids field on alert documents from kibana.rac.alert.space_ids to kibana.space_ids * fixes kb-rule-data-utils package * update snapshots * remove references to kibana.rac.alert.space_ids and replace with kibana.space_ids in rule registry integration tests and apm integration tests * fix apm functional test snapshots * undo index name changes I made in apm integration test configs * update typedocs references to upstream, not local repo # Conflicts: # x-pack/test/rule_registry/security_and_spaces/tests/trial/update_alert.ts

…06023) * kind of working solution... need to fix types.. would be great if all of this could go in the authorization class but I don't think we have access to the spaceids when we generate the kibana security action strings? * update mapping type as array:true for space_ids field, fixes types, updates jest tests, adds integration tests * undo changes in alerting authz class * update snapshot for apm api integration test for rules writing alerts * fix apm integration tests * omit version and sequence from expected outcome * re-add space id after this code was moved in master * add another default space id to test * fixes bug to remove duplicate spaceids * add space ids filter to elasticsearch query, updates detection role * update snapshot * update type docs for alerts client * remove dead code * fix type error * renames space ids field on alert documents from kibana.rac.alert.space_ids to kibana.space_ids * fixes kb-rule-data-utils package * update snapshots * remove references to kibana.rac.alert.space_ids and replace with kibana.space_ids in rule registry integration tests and apm integration tests * fix apm functional test snapshots * undo index name changes I made in apm integration test configs * update typedocs references to upstream, not local repo # Conflicts: # x-pack/test/rule_registry/security_and_spaces/tests/trial/update_alert.ts

elasticmachine · 2021-09-01T15:49:01Z

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

dhurley14 self-assigned this Jul 11, 2021

dhurley14 force-pushed the spaceids_alerts_authz branch from ab2f789 to 43cdac3 Compare July 13, 2021 21:07

dhurley14 added Feature:RAC label obsolete release_note:enhancement Theme: rac label obsolete v7.15.0 v8.0.0 labels Jul 14, 2021

dhurley14 marked this pull request as ready for review July 14, 2021 20:00

dhurley14 requested review from a team as code owners July 14, 2021 20:00

dhurley14 commented Jul 14, 2021

View reviewed changes

x-pack/test/apm_api_integration/configs/index.ts Outdated Show resolved Hide resolved

dhurley14 added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 15, 2021

xcrzx reviewed Jul 15, 2021

View reviewed changes

packages/kbn-rule-data-utils/src/technical_field_names.ts Outdated Show resolved Hide resolved

dhurley14 added 15 commits July 15, 2021 13:50

kind of working solution... need to fix types.. would be great if all…

5490056

… of this could go in the authorization class but I don't think we have access to the spaceids when we generate the kibana security action strings?

update mapping type as array:true for space_ids field, fixes types, u…

4cfd5d2

…pdates jest tests, adds integration tests

undo changes in alerting authz class

2c75cdf

update snapshot for apm api integration test for rules writing alerts

3def751

fix apm integration tests

8419531

omit version and sequence from expected outcome

55ef42c

re-add space id after this code was moved in master

4daa1dc

add another default space id to test

c709cea

fixes bug to remove duplicate spaceids

cd358c1

add space ids filter to elasticsearch query, updates detection role

c3cb624

update snapshot

9bbc14f

update type docs for alerts client

0b6e6ee

remove dead code

7067443

fix type error

544bb60

renames space ids field on alert documents from kibana.rac.alert.spac…

58be86e

…e_ids to kibana.space_ids

dhurley14 force-pushed the spaceids_alerts_authz branch from 0aeb067 to 58be86e Compare July 15, 2021 17:56

fixes kb-rule-data-utils package

896d304

dhurley14 added 3 commits July 15, 2021 18:23

update snapshots

add73d8

remove references to kibana.rac.alert.space_ids and replace with kiba…

93b858c

…na.space_ids in rule registry integration tests and apm integration tests

fix apm functional test snapshots

bd6c938

xcrzx approved these changes Jul 16, 2021

View reviewed changes

ymao1 reviewed Jul 16, 2021

View reviewed changes

x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md Outdated Show resolved Hide resolved

ymao1 reviewed Jul 16, 2021

View reviewed changes

x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts Show resolved Hide resolved

ymao1 approved these changes Jul 16, 2021

View reviewed changes

ogupte approved these changes Jul 16, 2021

View reviewed changes

dhurley14 added 2 commits July 16, 2021 13:35

undo index name changes I made in apm integration test configs

48e1b91

update typedocs references to upstream, not local repo

f6e6b92

dhurley14 enabled auto-merge (squash) July 16, 2021 17:56

dhurley14 merged commit dadeb78 into elastic:master Jul 16, 2021

marshallmain reviewed Jul 16, 2021

View reviewed changes

dhurley14 mentioned this pull request Jul 16, 2021

[7.x] [RAC] [RBAC] add space ids array to each alert document (#105173) #106023

Merged

peluja1012 mentioned this pull request Jul 20, 2021

[RAC] [Alerts] [RBAC] #95721

Closed

13 tasks

KOTungseth added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAC] [RBAC] add space ids array to each alert document #105173

[RAC] [RBAC] add space ids array to each alert document #105173

dhurley14 commented Jul 10, 2021 •

edited

Loading

xcrzx left a comment

xcrzx Jul 16, 2021

ymao1 left a comment

ogupte left a comment

kibanamachine commented Jul 16, 2021

API count

kibanamachine commented Jul 16, 2021

marshallmain Jul 16, 2021

dhurley14 Jul 16, 2021

dhurley14 Jul 16, 2021 •

edited

Loading

elasticmachine commented Sep 1, 2021

[RAC] [RBAC] add space ids array to each alert document #105173

[RAC] [RBAC] add space ids array to each alert document #105173

Conversation

dhurley14 commented Jul 10, 2021 • edited Loading

Summary

Checklist

For maintainers

xcrzx left a comment

Choose a reason for hiding this comment

xcrzx Jul 16, 2021

Choose a reason for hiding this comment

ymao1 left a comment

Choose a reason for hiding this comment

ogupte left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 16, 2021

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

kibanamachine commented Jul 16, 2021

💔 Backport failed

marshallmain Jul 16, 2021

Choose a reason for hiding this comment

dhurley14 Jul 16, 2021

Choose a reason for hiding this comment

dhurley14 Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Sep 1, 2021

dhurley14 commented Jul 10, 2021 •

edited

Loading

dhurley14 Jul 16, 2021 •

edited

Loading