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

Refactor Saved Object Client wrappers #133835

Closed
jportner opened this issue Jun 7, 2022 · 2 comments · Fixed by #142878
Closed

Refactor Saved Object Client wrappers #133835

jportner opened this issue Jun 7, 2022 · 2 comments · Fixed by #142878
Assignees
Labels
chore Feature:Saved Objects Feature:Security/Object Level Security Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature Meta Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@jportner
Copy link
Contributor

jportner commented Jun 7, 2022

Overview

The current architecture of the Saved Object Client doesn't meet the needs we have in upcoming enhancement requests.

Today, any plugin can register a Saved Objects Client (SOC) wrapper and specify the priority in which it should be applied. In practice, this is only used by the Platform Security team. The wrapper can modify both the input and the output of a given API call.

Consumers can either get an instance of the raw Saved Objects Repository (SOR), or they can get a scoped Saved Objects Client (and optionally omit any individual wrappers). Here's what the sequence looks like today when a consumer calls a SOC API:

source

It's also important to know that the introduction of the Spaces SOC wrapper was intentionally designed to be transparent to the consumer, and thus any scoped API call within a given space was intended to only function on objects that exist in that space. For example, if "dashboard 123" existed in Space A, but you are using the client in Space B, any searches or other API actions should not turn up that object.

This paradigm served us well in the past, but several things have changed:

  1. Most plugins (including spaces, security, and encryptedSavedObjects) can no longer be disabled as of the 8.0 release (By default, a plugin should not be disable-able #89584).
  2. To support shareable and "share-capable" saved objects (Sharing saved-objects in multiple spaces #27004), we had to remove the space ID from the serialized saved object document's raw ID; as a result, we had to add "preflight" checks in the Repository layer for any destructive actions (e.g., create+overwrite, update, or delete) to make sure that the target object existed in the current space before we modify it.
  3. To support object-level security (Object Level Security, Phase 1 #82725) -- which will enable things like private saved objects -- we will also have to conduct a "preflight" check in the Security wrapper layer to ensure that the object is owned by the person in question before. However, we want to avoid introducing an additional round-trip to Elasticsearch for performance reasons (Investigate risk of performance regression from share-capable saved object types #113743).
  4. Some consumers want the option to use a SOC that is not scoped to a particular space (Allow the SavedObjectsClient to work outside of a space #131254). Certain APIs already have options that enable cross-space functionality, but the SOC is not fully functional in a space-agnostic manner. To do this, we will have to have a Repository option that can essentially instruct it to (a) disable the functionality of the Spaces wrapper, and (b) disable its own deserialization checks that ensure an object exists in a given space.

We would be better served by moving away from the generic "wrapper"-style design pattern and implementing a bespoke model like this -- note this diagram includes future enhancements for OLS (3 above) but it does not include changes for space-agnostic SOC (4 above):

source

Technical changes

To implement this, we would need to make the following changes:

  1. Define the interfaces for Spaces, Security, and Encryption APIs in the Core codebase
  2. Replace the SOC wrapper registration function with bespoke functions for each of these three APIs
  3. Add the three new APIs with unit tests (borrowing code from existing SOC wrappers)
  4. Change SOR to use the new APIs, and update SOR unit tests
  5. Remove the SOC wrappers
  6. Retain existing integration tests (they should not need to be changed)
    • Edit: because we are removing the encapsulation that we had with the wrappers, I realized during my POC that it was easier to make one small change to authorization behavior. We should allow the SOR to do its basic validation (type, options, etc.) before attempting authZ checks. That means that integration tests need to change slightly, but they will be more consistent than before (because "superuser" was authorized for nonexistent object types, we had to have special test cases for those -- not anymore!)
@jportner jportner added chore Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Saved Objects Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature Feature:Security/Object Level Security labels Jun 7, 2022
@elasticmachine
Copy link
Contributor

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

@jportner jportner self-assigned this Jun 7, 2022
@legrego legrego added the Meta label Jun 8, 2022
@legrego legrego assigned jeramysoucy and unassigned jportner Jul 19, 2022
jeramysoucy added a commit that referenced this issue Nov 18, 2022
Merges the changes of #134395 into the new packages structure.
Resolves #133835

### Description
This PR represents a fully manual merge of the saved objects refactor of
client wrapper system into repository extensions. These changes are
being manually merged due to significant changes of the saved objects
implementation in the main branch, specifically the migration to the new
packages structure.

### Other changes
- Bulk Delete: bulk delete was implemented in parallel to #134395 being
completed and this PR will refactor that API to utilize the new
extensions

Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
@pchakour
Copy link

pchakour commented Jul 2, 2024

Hello !

I created some plugins that use SOC wrapper in order to change some visualization references, namespaces, patching visualization attributes, ...

I want to know the best way for me to migrate my SOC wrappers knowing that the bespoke functions are already implemented by x-pack :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Saved Objects Feature:Security/Object Level Security Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature Meta Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants