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.

@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 <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 60.60606% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.50%. Comparing base (dcbaa1a) to head (a86433b).

Files with missing lines Patch % Lines
...nsearch/security/resources/ResourcePluginInfo.java 0.00% 10 Missing ⚠️
...ch/security/resources/sharing/ResourceSharing.java 68.00% 6 Missing and 2 partials ⚠️
...ain/java/org/opensearch/sample/SampleResource.java 63.63% 4 Missing ⚠️
...arch/security/resources/ResourceAccessHandler.java 77.77% 1 Missing and 1 partial ⚠️
.../actions/rest/create/CreateResourceRestAction.java 75.00% 0 Missing and 1 partial ⚠️
...earch/security/spi/resources/ResourceProvider.java 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5735      +/-   ##
==========================================
+ Coverage   73.49%   73.50%   +0.01%     
==========================================
  Files         438      438              
  Lines       26744    26801      +57     
  Branches     3960     3974      +14     
==========================================
+ Hits        19655    19700      +45     
- Misses       5203     5215      +12     
  Partials     1886     1886              
Files with missing lines Coverage Δ
...org/opensearch/sample/SampleResourceExtension.java 100.00% <100.00%> (ø)
...arch/security/resources/ResourceIndexListener.java 96.72% <100.00%> (+0.16%) ⬆️
...i/migrate/MigrateResourceSharingInfoApiAction.java 78.57% <ø> (ø)
.../actions/rest/create/CreateResourceRestAction.java 78.78% <75.00%> (-0.53%) ⬇️
...earch/security/spi/resources/ResourceProvider.java 33.33% <50.00%> (+33.33%) ⬆️
...arch/security/resources/ResourceAccessHandler.java 73.64% <77.77%> (-0.48%) ⬇️
...ain/java/org/opensearch/sample/SampleResource.java 70.37% <63.63%> (+0.15%) ⬆️
...ch/security/resources/sharing/ResourceSharing.java 79.62% <68.00%> (-2.26%) ⬇️
...nsearch/security/resources/ResourcePluginInfo.java 79.83% <0.00%> (-7.33%) ⬇️

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