Making type required for SavedObjectsClient.find

[kobelb]

All usages of the SavedObjectClient#find currently specify the type, and the ability to secure #find without specifying the type requires us to be able to enumerate all known saved object types which isn't currently available.

This PR makes type required and throws an explicit error if it's used without specifying the type. There was potential for us to change the method signature of #find and move type to be a separate parameter, making this requirement more obvious to consumers, but given the fact that we'd like to support this long term, this seemed like an unnecessary change.

[rhoboat]

No current usages of the SavedObjectClient#find don't specify the type

I don't know if I understand that... you mean no current use specifies type? or all current use specifies type?

Edit: Oh, we're now requiring type, so none of them today do it already.

[rhoboat]

I'm not 100% on the reasoning behind this. Is there an issue or discussion about it?

[kobelb]

I don't know if I understand that... you mean no current use specifies type? or all current use specifies type

Sorry, really poor wording... All usages of SavedObjectClient#find specify the type. I edited the description to hopefully make the wording more obvious.

[kobelb]

It's to enable this: https://github.com/elastic/kibana/pull/18645/files#diff-f75e9a376a3b5fbbb3c95b0c7e28a269R47

We need to perform an authorization check on the #find method that constrains the user to the SavedObjectTypes they're able to authorized to find. The easiest way is to only support find for a single-type, which is what this PR does.

The other potential solution would be for us to know about all of the saved object types, determine which ones the user is authorized to "find" and then add a filter to the #find to constrain the "#find" to only authorized types. We don't currently have a way to enumerate all of the known saved object types, so I'm proposing the easiest solution in the short-term until we have this ability.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[kobelb]

Jenkins, test this

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[kobelb]

@archanid have you had a chance to put any more thought into this?

@elastic/kibana-platform care to weight in? RBAC Phase 1 is contingent on this solution or the alternate discussed in the issue description.

[rhoboat]

Thanks for more explanation around this. LGTM

[spalger]

I'm fine with it and in general prefer to limit the options we expose to the ones we need, Since we aren't using cross-type _find I think it's fair to remove it, but I'd like to hear from @epixa or @tylersmalley.

[epixa]

I'm not opposed to making type required for the sake of being pragmatic right now if it isn't being used elsewhere, but I do think we'll probably want the ability to search for saved objects in bulk without a type at some point to support things like backing up and restoring a space, for example. Just something to keep in mind.

[kobelb]

I'm not opposed to making type required for the sake of being pragmatic right now if it isn't being used elsewhere, but I do think we'll probably want the ability to search for saved objects in bulk without a type at some point to support things like backing up and restoring a space, for example. Just something to keep in mind.

Completely agree, and once we're able to enumerate all of the saved object types (hopefully enabled by the new saved objects system) then enabling this becomes trivial. This is just the path of least resistance at the moment.

[tylersmalley]

Would it be possible to constrain the results another way? As Court mentioned, this will limit or complicate our ability to provide things like a universal search across all types.

Since #17426 was merged, I also believe we are using find without a type since the listing includes mixed types? @chrisronline, can you confirm.

[kobelb]

#17426 added the ability to "find" multiple types, which we can support as well. I'm working on modifying this PR to support this approach as well.

[tylersmalley]

Spoke with @kobelb over Zoom. The SavedObjectClient is aware of the mapping, which you can use to get a list of all possible "types". This will allow us to get a filtered list of documents based on the allowed types.

[chrisronline]

@tylersmalley Yea, I added support for fetch multiple types in a single request

[rhoboat]

I look forward to catching up with the new changes to this PR and "re-approving."

[kobelb]

@tylersmalley was kind enough to point out that we have a way to enumerate the various types via the mappings, so we don't need to impose this limitation anymore.