Event log authorization on additional saved objects

[mikecote]

Follow up from: #55640 (comment).

The event log allows to link one event to zero/one/many saved objects. The query API requires a single saved object to be passed in and does authorization checks up front but we may need to handle authorization on the extra saved objects that may be part of an event.

Questions

1. How do we handle event logs with unauthorized access to some saved objects?

• Return a 403 response?
• Redact some of the information?
• Remove events with unauthorized access? (would case table pagination issues)

2. How do we handle objects that are deleted? We can't see if the user had authorized access to it or not.

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[mikecote]

For question 1, I'm thinking we keep it simple and fail the entire request (403). There shouldn't be a scenario the user doesn't have access to actions but has access to alerts.

Though, the opposite direction will have issues (actions referencing alerts the user doesn't have access to) but we can defer further changes and re-explore question 1 then.

[gmmorris]

For question 1, I'm thinking we keep it simple and fail the entire request (403). There shouldn't be a scenario the user doesn't have access to actions but has access to alerts.

Though, the opposite direction will have issues (actions referencing alerts the user doesn't have access to) but we can defer further changes and re-explore question 1 then.

I can see arguments for omitting as well once this is used wider than Alerting, which I feel should be out thinking in relation to EventLog.

Perhaps we can make this configurable (a strictness level config controlled as a query param) or perhaps a separate API endpoint? /_find would omit, while /_get would throw? Something to that effect.

[mikecote]

I can see arguments for omitting as well once this is used wider than Alerting, which I feel should be out thinking in relation to EventLog.

I'm usually not a fan of omitting just because it would cause weird pagination behaviour in a data table. One page may return 3 events, the next would return 9 events, the next 0 events, etc.

[pmuellr]

Another thing to think about is the consumer field for alerts. We currently don't save this in the event log documents, but perhaps we could (where appropriate), and then use that as an additional up-front query parameter.

[mikecote]

Another thing to think about is the consumer field for alerts. We currently don't save this in the event log documents, but perhaps we could (where appropriate), and then use that as an additional up-front query parameter.

Agreed

---

The solution for this should follow the same authorization mechanism developed in #63961.

[pmuellr]

Here's a route I'm trying, which seems promising.

Add a new field in the saved_objects nested object called rel. Meant to be vague-ish, named after the equally vague <link rel=blort> attribute. The only legal values are not set, and primary. This attribute will be set for event documents where the primary focus is on that saved object; the action for action execution, the alert for alert execution, and most importantly, only the alert for the alerting executeAction document; the action saved object in that document will NOT have the rel attribute set:

kibana/x-pack/plugins/alerting/server/task_runner/create_execution_handler.ts

Lines 92 to 103 in 6bf0e73

	const event: IEvent = {
	event: { action: EVENT_LOG_ACTIONS.executeAction },
	kibana: {
	alerting: {
	instance_id: alertInstanceId,
	},
	saved_objects: [
	{ type: 'alert', id: alertId, ...namespace },
	{ type: 'action', id: action.id, ...namespace },
	],
	},
	};

Then change the query functionality to only return documents with the requested saved object with the rel=primary field set.

What this means today is that the search for action saved objects will no longer return those alerting executeAction documents, only action execution documents, which don't reference the alert saved object (today). Which is the behavior we want - if you have an alert saved object, you should be able to see what actions it executed. But if you have an action saved object, you won't know what alerts executed it - you will know that it executed though.

In general, this rel=primary provides some scoping, so you can reference a saved object in a document, but not make that document findable with just that saved object reference. Only the "primary" saved objects can be found.

This might still not be strict enough - should we restrict who can see action execution? Though the plan would be to add rel=primary to action execution documents, we could just not do that, and you'd never be able to search for them.

Side note: it's becoming clearer to me that we will likely want some kind of new role to allow more general queries through the event log - for admin-type folks, in secure environments. So I'm not terribly concerned that by default action execution might not be viewable without elevated privileges. The data we store today for alert and action execution is ... not much ... an error message ... but we'd like to store more things like action execution result data, alert state, alert instance state, etc.

[mikecote]

The approach makes sense to me, it will secure things until we need to expose logs referencing saved objects that aren't the primary one (if ever that's the path we go in the future).

This might still not be strict enough - should we restrict who can see action execution?

I think that's ok, is there any information stored that the user shouldn't see? If not, that's fine.

[pmuellr]

This might still not be strict enough - should we restrict who can see action execution?

I think that's ok, is there any information stored that the user shouldn't see? If not, that's fine.

Yeah, we're fine today, but as soon as we add action/alert executor input/results/etc to the event log, we'll have to revisit.

[mikecote]

Yeah, we're fine today, but as soon as we add action/alert executor input/results/etc to the event log, we'll have to revisit.

👌 that's true, I've made a comment here: #63755 (comment) to keep that in mind.