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

Allow the SavedObjectsClient to work outside of a space #131254

Open
jportner opened this issue Apr 29, 2022 · 8 comments
Open

Allow the SavedObjectsClient to work outside of a space #131254

jportner opened this issue Apr 29, 2022 · 8 comments
Labels
discuss enhancement New value added to drive a business result Feature:Saved Objects Feature:Security/Spaces Platform Security - Spaces feature Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! triage_needed

Comments

@jportner
Copy link
Contributor

jportner commented Apr 29, 2022

Background

The SavedObjectsClient (SOC) was designed to work in the current space (aka "active space") by default. Consumers didn't need to specify what space they were in, it is determined by the HTTP request context which they use to obtain an instance of the SOC. The spaces SOC wrapper determines the active space, and sets the namespace option accordingly.

Over time we've had the need to change some SOC functions so they can work in multiple spaces. This includes:

With the exception of find/openPointInTimeForType/createPointInTimeFinder, the SavedObjectsClient always operates within the active space. Even for those other functions that can work in multiple spaces, the authorization layer still ensures that the user is authorized in the current space.

The remaining SOC functions only operate in the current space:

  • checkConflicts
  • get
  • resolve
  • bulkResolve
  • update
  • delete
  • collectMultiNamespaceReferences
  • updateObjectsSpaces
  • removeReferencesTo
  • incrementCounter

Feature

It would be useful to have the option to use the SavedObjectsClient in a space-agnostic manner. For example, this could be used to drive the ML Job Management page so you can manage jobs across all spaces that you have access to (#131134).

Unfortunately it's not trivial to change the entire SOC to behave in this manner, because some saved object types are still isolated to an individual space (namespaceType: 'single') and they do not have globally unique IDs.
I can think of three approaches to solve the ML Job Management use case.

Approach 1

ML jobs use namespaceType: 'multiple' and they do have globally unique IDs. We could potentially allow consumers to instantiate a SOC that works in a space-agnostic manner, but only for object types that have globally unique IDs. If a SOC was instantiated in this way, it would need to throw an error if a consumer attempted to use it for an isolated object type.

This would entail:

  • Changing the base SOC to allow instantiating it with this option
  • Changing the saved objects repository (SOR) to behave differently when conducting preflight checks and deserializing objects
  • Changing the SOR to force consumers to specify a namespace (or namespaces) when creating new objects
  • Changing the security SOC wrapper to handle authZ checks differently

Since the security SOC wrapper doesn't have access to the preflight check results that the SOR makes, it would have to check that the user is authorized to take that action in all spaces. This limitation would mean that users who are not "global admins" (e.g., they only have access to Kibana features in specific spaces) would never be authorized to make requests using this SOC.

Approach 2

This is a variation of Approach 1. If the security SOC wrapper had access to preflight check results, it could check authorization after the preflight check but before calling the base SOC function. We don't have the capability to do this today, but we'll likely need it anyway to implement #82725.

Approach 3

Alternatively, to support use cases like this one in the short term, the consumer could (1) create a SOC without the Spaces wrapper, (2) find objects across '*' spaces, and (3) take any subsequent update/delete actions in the context of a space where that object actually exists by specifying the namespace option.

One upside of this approach is that it only requires minimal changes to the SOC. Specifically the security SOC wrapper's find function would need to be tweaked to expand '*' to the list of actual spaces that exist, so that it can subsequently check authorization.

The ShareToSpaceFlyout logic would also need to be tweaked to allow the consumer to specify a custom API endpoint for collectMultiNamespaceReferences (AKA getShareableReferences).

A downside of this approach is that it's susceptible to race conditions. For example:

  • User loads a page with all ML jobs
  • One job "foo" exists in spaces "A" and "B"
  • A second user unshares "foo" from space "A" -- now it only exists in space "B"
  • The first user tries to make a change to "foo" -- it attempts to do this in the context of space "A" (because it has to specify a space, so the client picks the first space that the job currently exists in). However, the job no longer actually exists in space "A" so the user would see a 404 error.
@jportner jportner added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! enhancement New value added to drive a business result Feature:Saved Objects Feature:Security/Spaces Platform Security - Spaces feature labels Apr 29, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@jportner jportner added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Apr 29, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor

pgayvallet commented May 2, 2022

I encountered the same problem in #109196 for the SO management section. I needed to list/search for objects in all spaces (doable), but then there wasn't a way to properly get / update objects listed from different spaces.

I want to start by expressing a concern here. If your linked issue (ML management) and my use case (SO management) are prime and valid reasons to find a way to expose a 'non-spaced' client, I'm a bit scare of it being used outside of management features, where it, ihmo, shouldn't.

Now, regarding our options. Even if I really hate it, our current SOR architecture doesn't really support such high level behavior modification as option 1) and 2) propose, which means that the 'space-agnostic' mode will have to be directly implemented in the (already too complex) SOR code. Also, atm we don't provide options (from the SOR) to the SO wrapper when instantiating them, so it would be an additional structural change to implement if we wanted to try to handle that at a wrapper's level.

Overall, option 1) and 2) would require to take the time to tackle #102957 (and update the issue to support what would be required for the current issue). If you think this will be required anyway for #82725, then I'd say we should probably focus on the changes required for OLS first, and, once implemented, potentially perform the additional changes required for this issue.

In the meantime, if this is really blocker for some management feature, I'd say we should implement option 3) in the meantime to provide a temporary solution/workaround to have all SOR APIs accept a namespace(s) parameter.

WDYT?

EDIT: Also, note that for SOM, only option 3) would really work, as we will be listing objects from all spaces of all namespaceType, meaning we'll need to manipulate isolated objects from another space, which is not supported with your suggested option 1) or 2) ( it would need to throw an error if a consumer attempted to use it for an isolated object type)

@TinaHeiligers
Copy link
Contributor

I'm not mad about changing the base SOR either and forsee forcing consumers to change their implementations will be met with heavy resistance if it's not going to benefit a wider audience.
Not just that though, changing the API's is a breaking change and, even if approved, can really only be rolled out in ~18 months. That's hardly the 'short-term' solution we're looking for here 😦 .

To echo #131254 (comment), since we need to do work for OLS anyway, I'm +1 with seeing how far option 2 gets us before making other large-scale changes and with option 3 as a shorter-term solution.

@jportner
Copy link
Contributor Author

jportner commented May 6, 2022

I want to start by expressing a concern here. If your linked issue (ML management) and my use case (SO management) are prime and validate reasons to find a way to expose a 'non-spaced' client, I'm a bit scare of it being used outside of management features, where it, ihmo, shouldn't.

@pgayvallet I agree, I think that largely, other potential future use cases we're talking about would be management-oriented (Alerting, for example).

Overall, option 1) and 2) would require to take the time to tackle #102957 (and update the issue to support what would be required for the current issue).

Yes, I should have stated explicitly, but I was thinking the same thing!

If you think this will be required anyway for #82725, then I'd say we should probably focus on the changes required for OLS first, and, once implemented, potentially perform the additional changes required for this issue.

Yeah, I am pretty certain at this point it will be required for OLS anyway, and I agree we should focus on that first before tackling approach 1/2 in this issue.

In the meantime, if this is really blocker for some management feature, I'd say we should implement option 3) in the meantime to provide a temporary solution/workaround to have all SOR APIs accept a namespace(s) parameter.

WDYT?

We discussed it with the ML team and we think we might be able to put in the groundwork for option 2 (changes to optimize preflight checks) in the next few months. So they are OK with waiting for now, and we'll avoid making any short-term changes (option 3).

EDIT: Also, note that for SOM, only option 3) would really work, as we will be listing objects from all spaces of all namespaceType, meaning we'll need to manipulated isolated objects from another space, which is not supported with your suggested option 1) or 2) ( it would need to throw an error if a consumer attempted to use it for an isolated object type)

Hm, that's a good point. I guess if we implemented option 2, we could still change the SOM page to take advantage of it, but it would have to have some additional logic (on the client side and server side) to apply the correct space ID for single-namespace objects.

To echo #131254 (comment), since we need to do work for OLS anyway, I'm +1 with seeing how far option 2 gets us before making other large-scale changes and with option 3 as a shorter-term solution.

@TinaHeiligers 👍

@mattkime
Copy link
Contributor

There are two questions I'd like to be able to answer via data view management.

  1. Do any saved objects reference my data view?
  2. Which saved objects reference my data view?

#132385 adds saved object relationships to data view management. It would be nice to add cross space support to this.

We're not able to answer the first one without having some sort of visibility into all spaces. Seems this might be worth its own api endpoint since its really a boolean result.

@rudolf
Copy link
Contributor

rudolf commented May 18, 2022

For SOR to only work in a given space makes sense for the mental model where spaces are a kind of multi-tenant solution with strict isolation. Sharing to multiple spaces is stretching that mental model, and from my understanding ML's use case is starting to completely break away from that mental model into a new world where a user sees all objects from all their spaces.

These are all "management" use cases, but I wonder if it's more than just isolated use cases. I could also imagine us allowing users to see all saved objects across all the spaces they have access to (or only the ones they wish to see). If I want to see a marketing dashboard and then a sales dashboard why do I have to switch spaces and break my work flow.

I feel like we lack a strong product vision for how we see spaces being used and the kind of user experiences we want to build off of it. If ML is just the first adopter but other plugins will follow we would need built-in support rather than a workaround.

@jportner
Copy link
Contributor Author

We will lay the groundwork for Approach 2 in #133835 by getting rid of the SOC wrappers in favor of bespoke extensions. The PR to resolve that issue isn't ready to merge yet, but our team will pick it up soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New value added to drive a business result Feature:Saved Objects Feature:Security/Spaces Platform Security - Spaces feature Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! triage_needed
Projects
None yet
Development

No branches or pull requests

8 participants