Skip to content

Conversation

@markdboyd
Copy link
Contributor

@markdboyd markdboyd commented Aug 13, 2025

Description

These changes are a follow-up to #5491. They are part of ongoing work to address opensearch-project/alerting#1829. These changes specifically inject user custom attributes to the security plugin context so that DLS/FLS replacement on queries will work properly.

These changes work in concert with these changes in other plugins:

opensearch-project/common-utils#827
opensearch-project/alerting#1917

Issues Resolved

Addresses #5491

Testing

I have been doing manual testing in Docker by compiling the jar files and mounting the jar files into the containers.

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
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.09%. Comparing base (9ed37b5) to head (95e3a92).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...earch/security/privileges/PrivilegesEvaluator.java 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5560      +/-   ##
==========================================
+ Coverage   73.07%   73.09%   +0.01%     
==========================================
  Files         408      408              
  Lines       25259    25262       +3     
  Branches     3842     3843       +1     
==========================================
+ Hits        18459    18466       +7     
+ Misses       4933     4926       -7     
- Partials     1867     1870       +3     
Files with missing lines Coverage Δ
...va/org/opensearch/security/auth/RolesInjector.java 96.66% <100.00%> (+7.38%) ⬆️
...g/opensearch/security/privileges/IndexPattern.java 96.66% <100.00%> (-0.04%) ⬇️
...ensearch/security/privileges/TenantPrivileges.java 99.02% <100.00%> (-0.01%) ⬇️
...opensearch/security/privileges/UserAttributes.java 80.76% <100.00%> (+0.76%) ⬆️
...security/privileges/dlsfls/DocumentPrivileges.java 96.07% <100.00%> (-3.93%) ⬇️
...g/opensearch/security/support/ConfigConstants.java 95.23% <ø> (ø)
...earch/security/privileges/PrivilegesEvaluator.java 75.13% <0.00%> (ø)

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

@markdboyd markdboyd force-pushed the inject-user-custom-attributes branch 3 times, most recently from 4e2cf2e to b862081 Compare August 25, 2025 19:11
Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

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

Thank you for this PR @markdboyd! The changes LGTM. Left one suggestion on strengthening the logic to identify if attribute substitution for a query string is incomplete, but otherwise the changes look good. Thank you for all the contributions to solve this difficult bug in the alerting plugin!

@markdboyd markdboyd force-pushed the inject-user-custom-attributes branch from 68b84da to 5b5c8fe Compare August 26, 2025 14:19
… as a hashmap for improved compatibility with other plugins

Signed-off-by: Mark Boyd <[email protected]>
Signed-off-by: Mark Boyd <[email protected]>
Signed-off-by: Mark Boyd <[email protected]>
Signed-off-by: Mark Boyd <[email protected]>
… attribute substitution fails for DLS

Signed-off-by: Mark Boyd <[email protected]>
Signed-off-by: Mark Boyd <[email protected]>
@markdboyd markdboyd force-pushed the inject-user-custom-attributes branch from 5b5c8fe to 1a7c87d Compare August 26, 2025 18:33
cwperks
cwperks previously approved these changes Aug 26, 2025
Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

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

Thank you @markdboyd! The changes LGTM. left a few minor comments, but nothing major.

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.

thanks @markdboyd ! The PR looks good to me. left one clarifying question

@DarshitChanpura DarshitChanpura merged commit 7787bc2 into opensearch-project:main Aug 26, 2025
71 checks passed
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.

3 participants