Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SystemIndexRegistry from core to determine if request contains system indices #4471

Merged
merged 13 commits into from
Jul 11, 2024

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Jun 17, 2024

Description

The security plugin currently tracks system indices through an OpenSearch Setting. This PR keeps the behavior of the existing setting and also updates the SystemIndexAccessEvaluator to check if the request contains system indices using the new SystemIndexRegistry class from core.

Registered System Index - A registered system index is an index pattern that has been reserved by a plugin via the SystemIndexPlugin.getSystemIndexDescriptors extension point. Conventionally, system indices begin with a dot (.).

This PR adds an IT that demonstrates how the security plugin will determine if a request matches any system indices using the SystemIndexRegistry from core.

This is a step towards a solution for #4439, but more work needs to be done in core to convey which plugin has stashes the ThreadContext so the security plugin can properly authorize interactions with system indices.

  • Category

Enhancement

Issues Resolved

Related to #4439

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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 added 3 commits June 11, 2024 17:03
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
willyborankin
willyborankin previously approved these changes Jun 20, 2024
@cwperks
Copy link
Member Author

cwperks commented Jun 20, 2024

Setting this to Draft until Core PR is finalized and all associated PRs are merged.

@cwperks cwperks marked this pull request as draft June 20, 2024 18:05
cwperks added 2 commits June 20, 2024 14:39
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me. Left some comments. Will wait for CI to be green before re-reviewing

@cwperks cwperks changed the title Obtain registered system indices from core and add test that shows that core is passing the registered system indices Use SystemIndexRegistry from core to determine if request contains system indices Jul 9, 2024
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks cwperks marked this pull request as ready for review July 9, 2024 20:50
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.99%. Comparing base (9d32a8c) to head (5369120).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4471   +/-   ##
=======================================
  Coverage   64.99%   64.99%           
=======================================
  Files         314      314           
  Lines       22111    22112    +1     
  Branches     3566     3566           
=======================================
+ Hits        14370    14371    +1     
  Misses       5955     5955           
  Partials     1786     1786           
Files Coverage Δ
...earch/security/privileges/PrivilegesEvaluator.java 71.87% <100.00%> (ø)
...ecurity/privileges/SystemIndexAccessEvaluator.java 70.63% <100.00%> (ø)

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment, lgtm otherwise.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@DarshitChanpura DarshitChanpura added the backport 2.x backport to 2.x branch label Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants