Create a mechanism for plugins to explicitly declare actions they need to perform with their assigned PluginSubject

[cwperks]

Description

This PR allows plugins to define a plugin-permissions.yml file (available on the extending plugin's classpath) which contains the set of permissions it needs to run with outside of the regular system index access that plugins can already perform with their assigned PluginSubject.

For instance, the security plugin needs to be able to write to the auditlog index even though the auditlog index is not a system index. For this usecase, the plugin would add an index_permissions: section in its plugin-permissions.yml file that allows it to create and write to indices matching the security_auditlog* pattern.

This PR relates to strengthening system index access by deprecating try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { ... } which allow plugins to perform any action on the cluster in that block.

With the replacement, pluginSubject.runAs(() -> { ... } plugins are only limited to system index access for system indices that they formally register with SystemIndexPlugin.getSystemIndexDescriptors().

This PR gives plugins an additional mechanism for declaring other necessary actions they need to perform with their assigned subject.

• Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Enhancement

Issues Resolved

Replacement for opensearch-project/OpenSearch#15778 that does not require changes in the core

Related to: #4439

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.

[codecov]

Codecov Report

Attention: Patch coverage is 74.63768% with 35 lines in your changes missing coverage. Please review.

Project coverage is 72.42%. Comparing base (e80302b) to head (5127e5e).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...rg/opensearch/sample/utils/RunAsSubjectClient.java	57.14%	8 Missing and 1 partial ⚠️
...ecure/actions/rest/create/SecurePluginRequest.java	50.00%	7 Missing ⚠️
...cure/actions/rest/create/SecurePluginResponse.java	58.33%	5 Missing ⚠️
.../opensearch/security/OpenSearchSecurityPlugin.java	75.00%	2 Missing and 2 partials ⚠️
...actions/transport/SecurePluginTransportAction.java	87.50%	2 Missing and 1 partial ⚠️
...re/actions/rest/create/SecurePluginRestAction.java	83.33%	1 Missing and 1 partial ⚠️
...ecurity/privileges/SystemIndexAccessEvaluator.java	84.61%	0 Missing and 2 partials ⚠️
...ensearch/security/securityconf/impl/v7/RoleV7.java	84.61%	1 Missing and 1 partial ⚠️
...va/org/opensearch/sample/SampleResourcePlugin.java	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5341      +/-   ##
==========================================
- Coverage   72.43%   72.42%   -0.01%     
==========================================
  Files         388      394       +6     
  Lines       24177    24291     +114     
  Branches     3706     3713       +7     
==========================================
+ Hits        17512    17593      +81     
- Misses       4844     4868      +24     
- Partials     1821     1830       +9     

Files with missing lines	Coverage Δ
...secure/actions/rest/create/SecurePluginAction.java	100.00% <100.00%> (ø)
...in/java/org/opensearch/sample/utils/Constants.java	0.00% <ø> (ø)
...curity/identity/ContextProvidingPluginSubject.java	100.00% <100.00%> (ø)
...earch/security/privileges/PrivilegesEvaluator.java	74.85% <100.00%> (-0.15%)	⬇️
...va/org/opensearch/sample/SampleResourcePlugin.java	96.77% <85.71%> (-3.23%)	⬇️
...re/actions/rest/create/SecurePluginRestAction.java	83.33% <83.33%> (ø)
...ecurity/privileges/SystemIndexAccessEvaluator.java	76.87% <84.61%> (-0.06%)	⬇️
...ensearch/security/securityconf/impl/v7/RoleV7.java	91.78% <84.61%> (-5.19%)	⬇️
...actions/transport/SecurePluginTransportAction.java	87.50% <87.50%> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java	84.42% <75.00%> (-0.28%)	⬇️
... and 3 more

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nibix]

Generally, it looks good to me. I have left a few minor comments.

I have left one comment regarding mixing roles for normal users and for plugin users. I am wondering whether it could make sense to keep things a bit more strict apart by having two instances of ActionPrivileges: One for normal users, one for plugin users.

This would make any concerns about overlapping role names impossible. Additionally, the ActionPrivileges instance for plugin users would only be created once and could stay constant during the rest of the node life-time.

[cwperks]

I am wondering whether it could make sense to keep things a bit more strict apart by having two instances of ActionPrivileges: One for normal users, one for plugin users.

We can certainly do this. FYI I was also thinking of following a similar pattern for API Tokens (PR on my own fork atm which I plan to contribute to the feature branch on this repo) where there would be a convention that when authenticating with an api token that the "User" in the ThreadContext would start with a prefix like token:. With API Tokens, one of the goals is to associate permissions directly with the token (not roles) and those permissions would be stored in an index and follow similar pattern here where an in-memory "Role" is constructed and used for authz. I imagine that those permissions would also be stored in the same data structure and we can reconstruct one when necessary.

[nibix]

We can certainly do this. FYI I was also thinking of following a similar pattern for API Tokens (cwperks#56 which I plan to contribute to the feature branch on this repo)

Yes, it seems to be a common use case to have role-independent action privileges. It might make sense to have a look at the class wether we can refactor it in a way that role-independent action privileges (for API tokens and plugins) are easier to achieve. If you want, I can try to build a quick protoype for that.

[cwperks]

If you want, I can try to build a quick protoype for that.

Any help is appreciated :).

[cwperks]

@nibix I rebased this PR off of #5374

[nibix]

Minor nits, but already looks very good to me.

[DarshitChanpura]

left some nits. Looks good otherwise!