Skip to content

Conversation

@cwperks
Copy link
Member

@cwperks cwperks commented Oct 25, 2025

Description

In many plugin use-cases, they define relationships between a parent and child object such as a detector and the detector's results or a report definition and instances of a report.

This PR seeks to introduce a mechanism from ResourceProvider where a plugin can let security know about this relationship for the purpose of authorization.

For instance, in the Linux Filesystem read access to a folder gives a user read access to its descendants. Likewise, this PR seeks to introduce a mechanism where a user can be granted read access to a Report Definition and get access to read the associated instances.

The mechanism for this is through the Access Levels - i.e. action groups pertinent to a single sharable resource

For instance, consider the following pseudo definition for the access level to read a report definition

report-definition:
  report_definition_read_only:
    allowed_actions:
      - 'cluster:admin/opendistro/reports/definition/get'
      - 'cluster:admin/opendistro/reports/instance/get' # action pertinent to an instance defined on the access level of a report definition
      - 'cluster:admin/opendistro/reports/menu/download' # action pertinent to an instance defined on the access level of a report definition
  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Enhancement

Issues Resolved

Related to #4500

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.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks
Copy link
Member Author

cwperks commented Oct 25, 2025

Its important to note that this PR will not solve some other problems in relation to hierarchy:

  1. Grouping -> This PR does not consider cases where a child can move between parents (i.e. file moving between folders on the filesystem). In the case of the reporting plugin, a report instance is directly related to a report definition and that relationship cannot change (I suppose you can call it a sticky relationship)
  2. Searchable/Listable children -> In order for resource sharing to work properly, security needs to keep track of the all_shared_principals field on the resource documents themselves to support security injecting a DLS filter behind the scenes to ensure that users can only see what they are meant to see.
    • With parent/child relationships, if the reporting plugin needs to "List the most recent report instances", then it would require keeping track of all_shared_principals on the report instance documents in addition to the report definition. This would be possible by copying the report definition's sharing info to the report instance document on report instance creation, but its out of scope of this PR.

@cwperks
Copy link
Member Author

cwperks commented Oct 25, 2025

The changes in this PR seek to make security more embedded behind-the-scenes and ease integrating with security within other OpenSearch plugins. A step towards eliminating the need for the ResourceAccessControlClient.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 79.54545% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.07%. Comparing base (37c2d73) to head (87a93c0).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...ch/security/resources/sharing/ResourceSharing.java 75.34% 15 Missing and 3 partials ⚠️
...nsearch/security/resources/ResourcePluginInfo.java 73.68% 10 Missing and 5 partials ⚠️
...ain/java/org/opensearch/sample/SampleResource.java 69.23% 4 Missing ⚠️
...earch/security/spi/resources/ResourceProvider.java 33.33% 2 Missing ⚠️
...arch/security/resources/ResourceAccessHandler.java 84.61% 1 Missing and 1 partial ⚠️
...ecurity/resources/ResourceSharingIndexHandler.java 83.33% 2 Missing ⚠️
...pensearch/sample/SampleResourceGroupExtension.java 75.00% 1 Missing ⚠️
.../actions/rest/create/CreateResourceRestAction.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5735      +/-   ##
==========================================
+ Coverage   72.94%   73.07%   +0.12%     
==========================================
  Files         435      435              
  Lines       26542    26766     +224     
  Branches     3979     4016      +37     
==========================================
+ Hits        19360    19558     +198     
- Misses       5273     5283      +10     
- Partials     1909     1925      +16     
Files with missing lines Coverage Δ
...org/opensearch/sample/SampleResourceExtension.java 100.00% <100.00%> (ø)
...ava/org/opensearch/sample/SampleResourceGroup.java 50.00% <100.00%> (+7.14%) ⬆️
...va/org/opensearch/sample/SampleResourcePlugin.java 96.87% <ø> (ø)
.../transport/CreateResourceGroupTransportAction.java 75.00% <100.00%> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java 85.39% <100.00%> (+0.01%) ⬆️
...security/resources/ResourceActionGroupsHelper.java 53.84% <100.00%> (-2.26%) ⬇️
...arch/security/resources/ResourceIndexListener.java 96.55% <100.00%> (+0.63%) ⬆️
...i/migrate/MigrateResourceSharingInfoApiAction.java 76.30% <100.00%> (+0.99%) ⬆️
...ensearch/security/resources/sharing/ShareWith.java 75.00% <100.00%> (-8.34%) ⬇️
...pensearch/sample/SampleResourceGroupExtension.java 85.71% <75.00%> (-14.29%) ⬇️
... and 7 more

... and 15 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant