[RAC] integrating rbac search strategy with alert table

[XavierM]

Summary

We are integrating alert search strategy with RBAC on top of alert tables for security solution and o11y.

[elasticmachine]

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

[elasticmachine]

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

[yctercero]

LGTM! Think there's still a types issue, but other than that lgtm. Thanks for integrating!

[weltenwort]

Thanks for jumping in to fix this omission from the original RBAC PR 🙏

It works a bit differently from what I expected, though. I created alerts to test this and they showed up depending on whether I created them from the respective solution UI or the rules and connectors UI in the stack management section. Inspecting the code it seems the search strategy is filtering for kibana.rac.alert.owner. That field contains "logs", "apm" and so on when the alert was created through the solution UI, but "alerts" when created through the rules and connections UI.

I would have expected it to check kibana.rac.alert.producer, which corresponds to the rule type's owner. Why should there be a difference in whether it shows up in the alerts table depending on the place where the user created it?

Another aspect that made me wonder is why addition of a client-side request param (the event type) was needed to make the search strategy honor the users' capabilities. Couldn't this easily be tempered with by the user? I guess it has been designed like that for a reason, but I wanted to bring it up just in case. 😇

[yctercero]

Thanks for jumping in to fix this omission from the original RBAC PR 🙏

We wanted to keep these two things separate so hadn't added it to the original PR.

It works a bit differently from what I expected, though. I created alerts to test this and they showed up depending on whether I created them from the respective solution UI or the rules and connectors UI in the stack management section. Inspecting the code it seems the search strategy is filtering for kibana.rac.alert.owner. That field contains "logs", "apm" and so on when the alert was created through the solution UI, but "alerts" when created through the rules and connections UI.

We have a meeting set up for this I believe. @oatkiller is this going to be discussed as part of the breakout? We should include alerting folks in there as well. The owner field is set to be renamed to consumer, I'm not sure that it should be filtered by producer. We allow a consumer of the search strategy to specify which consumers to display.

I would have expected it to check kibana.rac.alert.producer, which corresponds to the rule type's owner. Why should there be a difference in whether it shows up in the alerts table depending on the place where the user created it?

Another aspect that made me wonder is why addition of a client-side request param (the event type) was needed to make the search strategy honor the users' capabilities. Couldn't this easily be tempered with by the user? I guess it has been designed like that for a reason, but I wanted to bring it up just in case. 😇

We added this because the logic for each is different. We have to use internal kibana user to query alerts and the asCurrentUser for non alerts. For the current release we aren't looking to create some sort of merged solution where we query each separately and merge and instead need to specify what we want. If you set the entity type to ALL it will query with asCurrentUser and the user will need to have ES privileges to the alerts index. This is mostly relevant for timeline which I don't think observability is using?

[weltenwort]

We wanted to keep these two things separate so hadn't added it to the original PR.

I can understand that - where can we see potential other follow-up tasks so we don't miss anything?

If you set the entity type to ALL it will query with asCurrentUser and the user will need to have ES privileges to the alerts index.

That's a good point. It escaped my attention that the two branches use different clients.

This is mostly relevant for timeline which I don't think observability is using?

Not sure what you mean by that. Due to the implementation of the t-grid data fetching we're using the timeline search strategy and Xavier's addition of the entityType parameter caused the execution to take the branch that uses the internal user. My concern was that the choice about which client to use is based on the browser-supplied parameter, which potentially opens the door for requests that exploit an inconsistency in the branch conditions. Not that I see one right now, but it feels risky to me in security-critical code. 😇

[weltenwort]

After talking to @XavierM I double-checked how the authorization matches up with what the user sees in the generic rule UI and the observability alerts table:

alerts table

rules UI

As mentioned before, this difference seems to stem from the usage of the "owner" (to renamed "consumer") field.

[jasonrhodes]

I think it's possible that a 7.15-friendly solution to this for now would be to make sure that the kibana.alert.consumers field is stored as an array, and the "producer" value from the alerting framework is added there for now, and when we decide exactly how we want this to work for rule sharing, we'll be able to add more values to that array later. Would that work for the RBAC filtering still, with minimal adjustments?

[weltenwort]

@jasonrhodes Assuming the result would be as intended, wouldn't it make more sense to achieve this by filtering on consumer OR producer in the RBAC filter? This wouldn't impact the indexed documents and would thereby be easier to adjust in future releases.

[jasonrhodes]

I'm thinking that in the future, we will want to have consumers be an array (and represent the consumers of these alerts, rather than the consumer of the rule, I think). In that way, I'd expect both the rule producer and the rule consumer to want to consume these alerts by default, and that later we may want to give users the option to remove one or the other (and add others, even). So the array with some default values feels like a future-proof solution to me.

[yctercero]

I'm thinking that in the future, we will want to have consumers be an array (and represent the consumers of these alerts, rather than the consumer of the rule, I think). In that way, I'd expect both the rule producer and the rule consumer to want to consume these alerts by default, and that later we may want to give users the option to remove one or the other (and add others, even). So the array with some default values feels like a future-proof solution to me.

I really don't think it makes sense to store an array of who should consume alerts in the data structure. I agree with @weltenwort that the functionality can be achieved with the existing solution. These fields are meant to denote authorization, not viewability preferences. I think that's the job of the query constructor (ie: us right now) to generate a query to filter as needed.

[weltenwort]

LGTM in light of what we discussed via a different channel today 👍 thanks for putting in the effort!

[michaelolo24]

LGTM!

[yctercero]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 6d9c91f
• Storybooks Preview

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
observability	261	262	+1
timelines	241	242	+1
total			+2

Async chunks

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

id	before	after	diff
observability	510.2KB	511.2KB	+1.1KB
securitySolution	6.5MB	6.5MB	-4.0B
timelines	272.0KB	273.1KB	+1.1KB
total			+2.2KB

Page load bundle

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

id	before	after	diff
infra	149.5KB	149.5KB	-4.0B
timelines	298.3KB	298.3KB	-65.0B
uptime	35.1KB	35.1KB	-4.0B
total			-73.0B

Unknown metric groups

API count

id	before	after	diff
timelines	927	928	+1

API count missing comments

id	before	after	diff
timelines	807	808	+1

History

• 💚 Build #143393 succeeded 3c6ba02
• 💚 Build #142827 succeeded d402d51
• 💚 Build #142750 succeeded 3c0cf2b
• 💚 Build #142062 succeeded 3c0cf2b
• 💔 Build #141834 failed 5da260e

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

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.