Investigate the ability to share a saved-object in all current and future spaces

[kobelb]

As currently envisioned, users are required to share saved-object in an explicit list of spaces. This does not allow users to share a saved-object in all current and future spaces by using a wildcard. We should investigate the complications that this introduces and figure out where it should come in our general phased implementation of sharing saved-objects in multiple spaces.

We should investigate moving saved objects from a "wildcard" share to/from a multi-namespace share.

Optionally, we should investigate (not necessarily implement) what it would take to support a deny-list of spaces (i.e., share to all spaces except marketing and sales)

[elasticmachine]

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

[kobelb]

/cc @elastic/ml-ui @droberts195

[jportner]

Saved object namespaces are mostly abstracted away from regular consumers: that is, consumers cannot modify namespaces outside of using the addToNamespaces and deleteFromNamespaces saved object APIs, and consumers cannot otherwise directly utilize namespaces in saved objects operations. However, there are two exceptions: as of 7.9 find allows you to search across multiple namespaces (#27003), and as of 7.10 (planned) bulkUpdate will allow you to update objects across multiple namespaces (#75449).

---

Hypothetically, we would identify "all spaces" as a '*' string, which is not currently a valid space ID. So an object that is shared to all spaces would look something like this:

{ "type": "foo", "id": "bar", "namespaces": ["*"] }

When a user edits a saved object's shared spaces in the UI (#75444), the client updates the saved object twice: it first adds any new namespaces, then it deletes any outdated namespaces. To use the object above as an example, if you wanted to "unshare" an object from all namespaces, the client would make two calls:

addToNamespaces({ type: 'foo', id: 'bar' }, namespaces: ['default']);
deleteFromNamespaces({ type: 'foo', id: 'bar' }, namespaces: ['*']);

Afterwards, the object would have the following namespaces:

{ "type": "foo", "id": "bar", "namespaces": ["default"] }

This and the inverse should work without any modifications.

Note: a core assumption is that if an object's namespaces contains '*', we should ignore any other explicitly-defined space.

---

With that in mind, the potential impacts of share-to-all-spaces / my proposed changes are:

SavedObjectsClient (general)

• When any operation is executed on a multi-namespace saved object, first it is fetched in a pre-flight get request and checked to ensure that it exists in the current namespace. It will be a small change to ensure that '*' is treated as the current namespace.
• Since Allow user to search saved objects across Spaces  #27003 was implemented, any operation returns a namespaces array for each non-namespace-agnostic saved object (even single-namespace saved objects). This is surfaced to external consumers (e.g. via the HTTP API routes for saved objects). I don't see any reason why we would need to change this.

SavedObjectsClient.create and SavedObjectsClient.bulkCreate

• We do not currently allow consumers to specify what namespace(s) they want an object to have during creation. We could easily change this by adding a new field to the Options for these methods -- however, if we do that, we may not want to expose it to the external HTTP route for the API.

SavedObjectsClient.delete

• Since Sharing saved-objects in multiple spaces: phase 1 #54043 was implemented, if the target is a multi-namespace object, the delete operation removes that object from the current namespace. If there are any namespaces remaining, the object is simply updated; otherwise, it is deleted. I can see four options for handling delete operations in a world where a saved object can be shared to all spaces:
  1. If the saved object exists in all spaces, delete it: If an object's namespaces contains '*' (any additional explicit namespaces would be ignored), it should just be deleted. This is my preference, though it could be viewed as inconsistent with how multi-namespace saved objects behave today.
  2. If the saved object exists in all spaces, delete it from the just the current space: This would involve expanding all spaces, removing just the current space from that array, and changing that saved object's namespaces to the new array. I don't like this because as soon as it happens, the object is no longer shared to all future yet-to-be-created spaces.
  3. Add support for a "deny-list": Change all saved objects operations to support a deny-list. If an object exists in all spaces and it is deleted in the current space, add it to that object's Spaces deny-list. This would allow an object to be deleted from the current space, but continue to exist in future spaces. I'm not sure that I like this approach, because I think a deny-list in general would unnecessarily complicate sharing, not to mention it sounds like potentially a lot of work for little benefit.
  4. Always delete the saved object: Alternatively, we could change delete to always delete an object -- but I suspect this is less desirable as it is a potentially unexpected saved object foot-gun for end users.

SavedObjectsClient.deleteByNamespace

• Since Sharing saved-objects in multiple spaces: phase 1 #54043 was implemented, multi-namespace objects will not be deleted outright, they will be updated to remove the target namespace (and only deleted if they have no remaining namespaces). We should not have to change this behavior.

SavedObjectsClient.find

• Since Allow user to search saved objects across Spaces  #27003 was implemented, consumers can search across all spaces with '*'. The Spaces saved objects client wrapper replaces this with actual space names. At the moment, if the Spaces plugin is disabled, the find operation treats '*' as the default namespace as a fallback. We shouldn't need to change this behavior.
  • Note: we won't provide a way for consumers to find only objects that exist in all spaces. I think this is okay, though.
• We will need to change getQueryParams (which find relies on under the hood) so that if a user searches with a specific namespace, the results also include objects that are shared in all spaces.

SecureSavedObjectsClientWrapper (general)

• If the Spaces plugin is disabled, AuthZ checks for any saved object are conducted against the global resource ('*'). We shouldn't need to change this behavior.
• If the Spaces plugin is enabled, AuthZ checks are conducted against a specific space. I can see three options for handling saved objects that are shared to all spaces:
  1. Check the user's permissions in the current space: The user must be authorized in the current space (i.e. scoped by the HTTP request). This is the simplest approach in my opinion. However, it could potentially cause some confusion; if a user is only authorized in "Space X", and they try to read a saved object that exists in all spaces using the "Space Y" API endpoint, they'll get an unauthorized error (even though they are authorized to read that object, they aren't authorized to read it in that context). FWIW, multi-namespace saved objects behave the same way today. This approach is my preference.
  2. Enumerate all of the spaces: The user must be authorized in at least one space. We could enumerate all spaces, check privileges for all of them, and ensure that we had at least one success. I don't like this approach, as it has some problems:
     • We have to do a preflight request inside of the SecureSavedObjectsClientWrapper to get the object's namespaces before we can figure out how to check authorization. This means extra unnecessary round trips to Elasticsearch and a not-insignificant amount of complexity being added to the SecureSavedObjectsClientWrapper.
     • This adds another (circular) dependency from the Security plugin to the Spaces plugin. We currently rely on this but it's not ideal.
     • To stay consistent, we would need to change AuthZ checks for saved objects that exist in multiple spaces to behave the same way.
  3. Always check the global resource: The user must be authorized in all spaces. I don't like this approach, as it contradicts how we treat saved objects that exist in multiple spaces.

SecureSavedObjectsClientWrapper.addToNamespaces and SecureSavedObjectsClientWrapper.deleteFromNamespaces

• For these operations, the user has to have create/read operations in all individual target namespaces, respectively. We will need to change this so it checks the global resource when '*' is used.

Other

• We'll need to make some minor UI Changes to represent '*' as a namespace. At a minimum this is the Saved Objects Management table (the "Shared spaces" column), and we will need to conditionally display this in the Share to Space flyout (if the user is authorized for the global resource).
• We shouldn't need to change other saved object / spaces APIs (get, bulkGet, update, bulkUpdate, import, export, copySavedObjectsToSpaces).

[kobelb]

Thanks for doing all this research @jportner, it's really great.

It will be a small change to ensure that '*' is treated as the current namespace.

This is purely a nitpick on the wording; however, in my opinion * correlates to all current and future spaces, so the user's current space isn't "all current and future spaces", the current space is an explicit space but the saved-object exists in all current and future spaces.

Since #27003 was implemented, any operation returns a namespaces array for each non-namespace-agnostic saved object (even single-namespace saved objects). This is surfaced to external consumers (e.g. via the HTTP API routes for saved objects). I don't see any reason why we would need to change this.

When we implemented #27003, what is returned for the namespaces property when a saved-object is in a namespace that the user doesn't have access to?

If the saved object exists in all spaces, delete it: If an object's namespaces contains '*' (any additional explicit namespaces would be ignored), it should just be deleted. This is my preference, though it could be viewed as inconsistent with how multi-namespace saved objects behave today.

I'd personally prefer option iv as it's the most consistent. I wonder if we could add a force option which must be specified when the user is deleting a saved-object and it exists in multiple spaces? This would potentially eliminate the foot-gun scenario while keeping this consistent.

Check the user's permissions in the current space: The user must be authorized in the current space (i.e. scoped by the HTTP request). This is the simplest approach in my opinion. However, it could potentially cause some confusion; if a user is only authorized in "Space X", and they try to read a saved object that exists in all spaces using the "Space Y" API endpoint, they'll get an unauthorized error (even though they are authorized to read that object, they aren't authorized to read it in that context). FWIW, multi-namespace saved objects behave the same way today. This approach is my preference.

Mine too!!

[jportner]

Thanks for doing all this research @jportner, it's really great.

😁

This is purely a nitpick on the wording; however, in my opinion * correlates to all current and future spaces, so the user's current space isn't "all current and future spaces", the current space is an explicit space but the saved-object exists in all current and future spaces.

I think we both agree on the same thing, but it's definitely important to make sure all audiences will have the same understanding 🙂 So maybe this is a better way to word it?

• When any operation is executed on a multi-namespace saved object, first it is fetched in a pre-flight get request and checked to ensure that it exists in the current namespace. It will be a small change to ensure that '*' is treated as the current namespace. It will be a small change to ensure that * also passes this check, since it includes the current namespace.

When we implemented #27003, what is returned for the namespaces property when a saved-object is in a namespace that the user doesn't have access to?

Well, if you try to find saved objects using namespaces that you aren't authorized for, the operation will fail with a 403 error.
Otherwise, if an object exists in a space you are authorized for and another that you aren't authorized for, the unauthorized namespaces are redacted (replaced with '?' characters).

I'd personally prefer option iv as it's the most consistent. I wonder if we could add a force option which must be specified when the user is deleting a saved-object and it exists in multiple spaces? This would potentially eliminate the foot-gun scenario while keeping this consistent.

That's an idea, but wouldn't that be considered a breaking change?

Mine too!!

👍

[kobelb]

When any operation is executed on a multi-namespace saved object, first it is fetched in a pre-flight get request and checked to ensure that it exists in the current namespace. It will be a small change to ensure that '*' is treated as the current namespace. It will be a small change to ensure that * also passes this check, since it includes the current namespace.

I like this wording better!

That's an idea, but wouldn't that be considered a breaking change?

I could see an argument being made in that regard... However, the delete API will continue to behave the same with saved-objects that can't or haven't been shared to multiple spaces. None of the suggestions are completely backward compatible, consistent and not a foot-gun. I'm interested in what you think @legrego

[jportner]

I could see an argument being made in that regard... However, the delete API will continue to behave the same with saved-objects that can't or haven't been shared to multiple spaces.

Assuming we did choose to implement the suggested `force option:

It will continue to behave the same way for the time being. But when we eventually convert single-namespace object types to multi-namespace object types (Dashboards, for instance), any consumers that currently rely on the delete API for these types may suddenly find their usage to be broken.

None of the suggestions are completely backward compatible, consistent and not a foot-gun. I'm interested in what you think @legrego

Agree on both of these points!

[legrego]

Thanks for the writeup @jportner!

Chatted with @jportner offline to assuage my concerns. I'm also in board with option iv:

Always delete the saved object: Alternatively, we could change delete to always delete an object -- but I suspect this is less desirable as it is a potentially unexpected saved object foot-gun for end users.

And I think @kobelb's suggestion of introducing the force option is a reasonable approach to take, too.

In terms of the authorization model, Check the user's permissions in the current space makes sense to me, and is most consistent with the rest of the operations we perform today

[jportner]

Great!

Now since you only need to have permissions in one space to update or delete a saved object, do you both think it makes sense to change the authorization checks for deleteFromNamespaces as well?

[legrego]

Now since you only need to have permissions in one space to update or delete a saved object, do you both think it makes sense to change the authorization checks for deleteFromNamespaces as well?

To clarify, the proposal is to ensure that the user is authorized in at least one space that the saved object exists in? (presumably the user's current space)

{
   "type": "index-pattern",
   "index-pattern": {},
   "namespaces": ["marketing", "sales", "default"]
}

In this example, a user with read/write access to the sales space (and no other spaces) could unshare it from the marketing and default spaces?

That makes sense to me, since they could just as easily delete it from the other spaces. It does potentially lead to information disclosure though, since they would first see the saved object as:

{
   "type": "index-pattern",
   "index-pattern": {},
   "namespaces": ["sales", "?", "?"]
}

The user could then try to brute-force an unshare by guessing space ids, and wait for one of the ? namespaces to disappear from the saved object.

Other than consistency, is there another reason to pursue this change?

[jportner]

To clarify, the proposal is to ensure that the user is authorized in at least one space that the saved object exists in? (presumably the user's current space)

{
   "type": "index-pattern",
   "index-pattern": {},
   "namespaces": ["marketing", "sales", "default"]
}

In this example, a user with read/write access to the sales space (and no other spaces) could unshare it from the marketing and default spaces?

That makes sense to me, since they could just as easily delete it from the other spaces. It does potentially lead to information disclosure though, since they would first see the saved object as:

{
   "type": "index-pattern",
   "index-pattern": {},
   "namespaces": ["sales", "?", "?"]
}

The user could then try to brute-force an unshare by guessing space ids, and wait for one of the ? namespaces to disappear from the saved object.

Yes to all of the above.

Other than consistency, is there another reason to pursue this change?

No, consistency would be the only reason IMO. It shouldn't affect the average user's experience, we won't provide a way to unshare from a space you don't have permission for via the UI. So we could leave this as-is, in light of the information disclosure you pointed out.

[legrego]

So we could leave this as-is, in light of the information disclosure you pointed out.

👍 this is my preference for now. We can revisit if we have a more compelling reason to allow this behavior.

[jportner]

• We shouldn't need to change other saved object / spaces APIs (get, bulkGet, update, bulkUpdate, import, export, copySavedObjectsToSpaces).

I'll amend this:

I think we need to change copySavedObjectsToSpaces, 1. change the UI to disable "legacy" copy for objects that exist in all spaces, and 2. change the API to omit referenced objects that already exist in all spaces (otherwise you could copy to multiple spaces and get multiple conflicts with the same destination object)

Closing this investigation issue as I'm starting the implementation already.