Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[alerting event log] add query support #55633

Closed
pmuellr opened this issue Jan 22, 2020 · 11 comments · Fixed by #62015
Closed

[alerting event log] add query support #55633

pmuellr opened this issue Jan 22, 2020 · 11 comments · Fixed by #62015
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@pmuellr
Copy link
Member

pmuellr commented Jan 22, 2020

Currently the event log only supports writing log entries, there is no way to read them back. We'll need to add that.

Note that we probably will want to have an http endpoint for this as well.

The current thinking is that to query the event log, you can only query for events associated with saved objects that you have access to, via those saved object ids. The ids are written to the log entries (added by the client writing log entries, eg the actions and alerting plugins). This is for security purposes. If you can see the saved object, then you should be able to see log entries associated with that saved object.

So, at a minimum, the query should take a list of saved objects as input to the query, but obviously we'll want some other query parameters like date range, pagination controls, and queryable fields in the event log entries themselves.

During some discussions with future clients of the event log, it's not clear if passing in a list of saved object id's is feasible in all cases. A client might want to get a list of all the events that they can "see", without having to pass in a list of saved object ids. That seems potentially do-able - internally, we could do a query over all event log entries, and for each one, get it's associated saved object ids, and "test" whether the user can "see" those saved objects, and allow those to be returned. The performance of that kind of processing seems like it won't be great.

@pmuellr pmuellr added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Jan 22, 2020
@elasticmachine
Copy link
Contributor

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

@pmuellr
Copy link
Member Author

pmuellr commented Feb 27, 2020

I think the telemetry added in PR #58081 will need to query the eventLog, and in it's use case, just needs action execution counts, not the actual event log documents. I think once the query support lands, we can then update the telemetry code to use this query.

@pmuellr
Copy link
Member Author

pmuellr commented Feb 28, 2020

I was going to start in on this, so assigned myself, realized it would be better to get the index threshold alert type working with the existing U, so unassigned myself.

@pmuellr
Copy link
Member Author

pmuellr commented Mar 26, 2020

Note that came up during an event log brain dump, was that a query should first do a GET on the relevant objects being searched (alert/action), to ensure the user actually has access to these objects, before searching for them. Otherwise, someone could get an alert/action id via some other means, and use it to look up the event history, even if they didn't have access to the alert/action object. If they did get access to an alert id, but didn't have access to the alert, this would presumably be handled as a security error or returning an empty response, without ever doing an actual query.

I guess this also raises a question about whether the "input" from the query should be "typed" to alert/action or generic to saved objects. Is it good enough to do a GET on the relevant saved object, or should we be trying to do a GET on the action/alert. Presumably the latter could have additional constraints than the former. But that means the query would need to have access to the alertingClient / actionsClient, or worse (try to access the objects via HTTP gets to the relevant endpoints). I guess this could impact the way we save the references as well, to make them more specifically "typed" than just saved object references. I hope we don't have to go this route ...

@mikecote
Copy link
Contributor

I guess this also raises a question about whether the "input" from the query should be "typed" to alert/action or generic to saved objects.

You're right, alerts (and maybe actions) themselves will need special handling. If we have to go down a special route, maybe a registry of custom handlers per SO type would work (to keep it flexible)? If there's no custom handler for a given SO type then we use the default SO access via savedObjectsClient.get(type, id);.

@gmmorris gmmorris self-assigned this Mar 30, 2020
@gmmorris
Copy link
Contributor

Looking at the underlying shape of event in the log, I'm reminded that an event log can have references to multiple saved objects.

When we get the events by the ID of a saved object, we'll need to validate the user's privileges for every single Saved Objects in every single event returned by the query.
It's likely these IDs repeat in multiple events, so we should only check the access for each individual ID once, but we must check for each unique one, obviously.

@mikecote
Copy link
Contributor

This may change with #55640.

I think the approach for query support would be to ensure the user has access to the saved object in context before doing a query to the event log. It would also append a filter to the query done on the index to return only objects where type & id exist in the array.

If we're concerned with the access of the other objects in the event log references array, we can tackle it with #55640.

@gmmorris
Copy link
Contributor

It would also append a filter to the query done on the index to return only objects where type & id exist in the array.

Hmm, interesting.
What's the thinking for this? My instinct was to filter out any object that the user doesn't have permissions to fetch, rather than just ensure it has a type/id.

@mikecote
Copy link
Contributor

I think we're on the same page but I might of responded too vague.

To clarify my response, I was referencing type & id as the values the query API would receive (ex: type alert and id 123 from GET /event_log/:type/:id/_find). We'll have to ensure the user has access to that saved object via savedObjectsClient.get(type, id) before doing the query and then inject a filter to the event log query that only returns logs related to that saved object.

@mikecote
Copy link
Contributor

@pmuellr

So, at a minimum, the query should take a list of saved objects as input to the query, but obviously we'll want some other query parameters like date range, pagination controls, and queryable fields in the event log entries themselves.

During some discussions with future clients of the event log, it's not clear if passing in a list of saved object id's is feasible in all cases. A client might want to get a list of all the events that they can "see", without having to pass in a list of saved object ids. That seems potentially do-able - internally, we could do a query over all event log entries, and for each one, get it's associated saved object ids, and "test" whether the user can "see" those saved objects, and allow those to be returned. The performance of that kind of processing seems like it won't be great.

I know this issue description hasn't been revisited in a while but I think to keep the initial implementation simple, we should make the API focus on a single saved object. I think with uncertainties of how the activity log view will work and what the direction of #55640 will be, we can enhance this API later.

This will probably give us enough to satisfy the alert details view and possibly the actions the alert executed.

Thinking of it, we should probably start taking a closer look at the discuss #55640 and see how we'll store object references.

Thoughts?

@pmuellr
Copy link
Member Author

pmuellr commented Mar 31, 2020

I think to keep the initial implementation simple, we should make the API focus on a single saved object.

yes, absolutely. For our immediate needs, the query could take a single SO reference. If we need a multi-SO search, we could enhance the args to take a single | array, or create a separate API endpoint - figure it out when we get there.

@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants