[Feature] Introduces Centralized Resource Access Control and Sharing

[DarshitChanpura]

Description

Introduces a new authorization mechanism to control access to resources defined by plugins.
There are 4 major components to this PR:

1. Introduces an SPI that defines the contracts for a resource sharing plugin as well as Resource Access Control plugin.
2. Introduces 4 new APIs that expose resource-sharing and access verification capabilities.
3. Modifies DLS to implicitly check for resource-access for requesting user if incoming request is for a resource
4. Adds a sample resource plugin with tests to demonstrate the usage of this new feature.
5. Adds a feature flag to toggle the feature. Feature is enabled by default.

• Category : New feature
• Why these changes are required?
  • At present, plugins have implemented in-house authorization mechanisms for a lack of centralized framework. This PR introduces a centralized way to offload resource permissions check to security plugin. Plugins will leverage the java APIs introduced in core.

Issues Resolved

• Related to [Proposal] Resource Permissions and Sharing #4500

Testing

• automated and manual testing

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[DarshitChanpura]

I would guess that this is only because the resource sharing index is not marked as hidden. Marking it as hidden should fix this issue without changing the observable behavior.

@nibix should the index be marked as hidden? On another note, this PR has been updated with integration test for sample plugin and these tests run with CI as well. Would you mind re-reviewing it? I'll be pushing the changes to fix code-cov CI.

[DarshitChanpura]

In case anyone needs help getting started with review, here is an order that you can follow.

1. Start with the SPI folder.
2. Review OpenSearchSecurityPlugin to understand how the SPI is utilized to setup resource index listeners.
3. Next, review resources package, particularly ResourceAccessHandler and ResourceSharingIndexHandler.
4. Next review DLS/FLS related changes.
5. Once you review the main package, you can then review Sample-resource plugin to understand how a plugin might utilize it and checkout the integration tests to understand two paths, i.e. feature flag enabled and disabled.

[cwperks]

nit: Similar to previous comment, wdyt about naming this SharableResourceExtension?

[DarshitChanpura]

IMO ResourceSharingExtension is more clear.

[cwperks]

Is this change necessary?

[nibix]

Please see the comment in #5016 (comment) ... i fear, the issue described there still holds.

While the method suggests that the call is "async", the join() call again blocks an OpenSearch thread which brings the risk of thread pool exhaustion and thus deadlocks. It just circumvents the assertion which is done in the actionGet() method.

I am sorry to say, but I am pretty confident that it is not safely possible to make search requests within low level operations like onPreQueryPhase or the lucene filter leaf readers. Even if it would be possible, the performance would be miserable, as this would cause the query to be fired thousands of times for a single user search.

I fear that a completely different approach is necessary:

• Filter level DLS was designed to allow DLS for term lookup queries which seems a thing which comes closest to the current approach.
• Alternatively, the whole resource sharing information must be kept in the Java heap.

Still, I have severe doubts whether DLS is really the correct way to tackle this issue.

Reasons:

• DLS does only cover read operations. It does not cover write operations and cannot be quickly extended to cover it. I guess, resource sharing, however, also needs support for write operations?
• I have doubts whether the equation "one resource is exactly one document" is really true. See also the question here: [Question] Should resource-sharing information be stored on a dedicated index or on the resource index inside the documents? #5014 (comment)

[cwperks]

DLS does only cover read operations. It does not cover write operations and cannot be quickly extended to cover it. I guess, resource sharing, however, also needs support for write operations?

I think with the approach @DarshitChanpura took in this PR, there is a separate index that security owns and keeps track of the resource_user (the owner of the resource) along with sharing info. Plugins reserve the right to write to their system indices, but if the index contains resources (as put forward in this feature) then behind-the-scenes security will filter the search request to only return resources pertinent to the authenticated user.

[DarshitChanpura]

The idea behind this approach was to prevent plugin dev errors. It was discussed as part of the PR in core. (comment). By implementing a DLS approach we do not require plugin devs to make any calls manually.

@reta @nibix @cwperks I think we need to align on the path forward. DLS or not? I've already introduced a sharing api as part of this PR.

[nibix]

@DarshitChanpura

should the index be marked as hidden?

yes, any system index which is not directly relevant to the end user should be not visible by default to the user. Thus, it should be marked as hidden.

[DarshitChanpura]

@DarshitChanpura

should the index be marked as hidden?

yes, any system index which is not directly relevant to the end user should be not visible by default to the user. Thus, it should be marked as hidden.

In this case the index is not technically directly relevant however when user shares the info or wants to check who has access to this resource. So I'm not sure if hidden still applies here.

[DarshitChanpura]

I've opened a discussion issue to finalize the approach. #5062