fixed throw exception for DLS user attributes by ThyTran1402 · Pull Request #5975 · opensearch-project/security

ThyTran1402 · 2026-03-01T16:46:23Z

Description

When a DLS query references a user attribute ${attr.jwt.array} that is not present in the authenticated user's JWT/custom attributes, UserAttributes.replaceProperties() leaves the placeholder in the query string. The guard in DocumentPrivileges.Dynamic.evaluate() correctly detects this and throws a PrivilegesEvaluationException, but the previous error message ("Invalid DLS query: ") gave operators no indication of which attribute was missing or what attributes were available — forcing manual inspection of both the query and the user's token.

Before: PrivilegesEvaluationException: Invalid DLS query: {"terms":{"arr":["${attr.jwt.array}"]}}
Fixed: PrivilegesEvaluationException: DLS query references undefined user attributes: [attr.jwt.array]. Available attributes are: [attr.jwt.roles, attr.proxy.department]

Issues Resolved

Add ability to change default DLS parameter substitution behavior #1310

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

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 · 2026-03-03T12:21:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.79%. Comparing base (7466266) to head (523ac56).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5975      +/-   ##
==========================================
- Coverage   73.81%   73.79%   -0.02%     
==========================================
  Files         439      439              
  Lines       27087    27110      +23     
  Branches     4018     4023       +5     
==========================================
+ Hits        19993    20007      +14     
- Misses       5188     5195       +7     
- Partials     1906     1908       +2

Files with missing lines	Coverage Δ
...opensearch/security/privileges/UserAttributes.java	`84.37% <100.00%> (+3.60%)`	⬆️
...security/privileges/dlsfls/DocumentPrivileges.java	`91.80% <100.00%> (+1.06%)`	⬆️

... and 10 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 · 2026-03-03T17:18:35Z

Thank you for this!

Mostly looks good to me.

Can you review the CI failures?

Mainly, you'd need to add an entry to CHANGELOG.md and run ./gradlew :spotlessApply once on the code. See https://github.com/opensearch-project/security/blob/main/CONTRIBUTING.md for some details on this.

ThyTran1402 · 2026-03-03T18:24:57Z

Thank you for this!

Mostly looks good to me.

Can you review the CI failures?

Mainly, you'd need to add an entry to CHANGELOG.md and run ./gradlew :spotlessApply once on the code. See https://github.com/opensearch-project/security/blob/main/CONTRIBUTING.md for some details on this.

Hi @nibix

I updated changelog. Can you review it please?

Thank you.

ThyTran1402 · 2026-03-11T14:49:21Z

Hi @cwperks

I think the PR is ready for review.

Thank you.

cwperks · 2026-03-11T14:58:00Z

@ThyTran1402 apologies for not being able to get comments to you yesterday. I will take another look shortly.

cwperks · 2026-03-11T15:04:56Z

Changes LGTM. Thank you for the PR @ThyTran1402 ! I'll get another maintainer to take a look as well.

cwperks · 2026-03-11T15:05:35Z

@ThyTran1402 one other thing, can you run ./gradlew spotlessApply and commit and push the formatting fixes? I see Code Hygiene check is failing.

cwperks · 2026-03-11T20:49:17Z

@ThyTran1402 FYI I pushed a commit to fix a conflict in the changelog as another pr just merged. If you need to make a change, please pull before pushing. Hopefully no more changes needed though bc the PR lgtm.

Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

ThyTran1402 · 2026-03-12T18:46:08Z

@ThyTran1402 FYI I pushed a commit to fix a conflict in the changelog as another pr just merged. If you need to make a change, please pull before pushing. Hopefully no more changes needed though bc the PR lgtm.

Hi @cwperks

I added unit test recently since I see it did not pass tests coverage 😢 .

Thank you!

Co-authored-by: Craig Perkins <craig5008@gmail.com> Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

ThyTran1402 · 2026-03-13T12:40:19Z

Are there anything that I need to do from my side to pass these tests? 😭 @cwperks
Since I think there's different version for plugin and it seems like flaky tests also.

cwperks · 2026-03-13T17:34:12Z

@ThyTran1402 no worries, those CI checks can be flaky and pass on re-run. I think test coverage is more than sufficient already in this PR.

rursprung · 2026-04-23T12:19:57Z

+    public static List<String> findUnresolvedAttributes(String s) {
+        List<String> result = new ArrayList<>();
+        Matcher matcher = UNRESOLVED_ATTRIBUTE_PATTERN.matcher(s);
+        while (matcher.find()) {


this whole method can be simplified to return UNRESOLVED_ATTRIBUTE_PATTERN.matcher(s).results().map(m -> m.group(1)).toList();
see Matcher#results

(stumbled over it by chance while reviewing a bug)

hmmm that's interesting. It seems to be cleaner with Matcher#results().
I wonder should I open another PR for this suggestion since it already merged?

i raised #6122

no functional change, this just simplifies the code. this is a follow-up to PR opensearch-project#5975 Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com>

ThyTran1402 requested review from DarshitChanpura, RyanL1997, cwperks, derek-ho, nibix, reta, shikharj05 and willyborankin as code owners March 1, 2026 16:46

cwperks reviewed Mar 4, 2026

View reviewed changes

Comment thread src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java Outdated

cwperks reviewed Mar 11, 2026

View reviewed changes

Comment thread src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java Outdated

cwperks previously approved these changes Mar 11, 2026

View reviewed changes

DarshitChanpura reviewed Mar 11, 2026

View reviewed changes

Comment thread src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java Outdated

Comment thread CHANGELOG.md

ThyTran1402 dismissed cwperks’s stale review via 47b7717 March 11, 2026 17:28

cwperks previously approved these changes Mar 11, 2026

View reviewed changes

nibix previously approved these changes Mar 12, 2026

View reviewed changes

RyanL1997 previously approved these changes Mar 12, 2026

View reviewed changes

ThyTran1402 dismissed stale reviews from RyanL1997, nibix, and cwperks via c559a22 March 12, 2026 18:35

ThyTran1402 added 3 commits March 12, 2026 14:40

fixed throw exception for DLS user attributes

5c77a2d

Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

updated changelog

e0373eb

Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

set the limit for amount of chars

4377cfb

Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

ThyTran1402 added 2 commits March 12, 2026 14:40

fixed hard coding limit and use text blocks

42dc139

Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

added unit test for more than 10 attributes

5a47c7b

Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

ThyTran1402 force-pushed the fix/improve_ability_dls_user_attributes branch from c559a22 to 5a47c7b Compare March 12, 2026 18:41

cwperks reviewed Mar 12, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Update CHANGELOG.md

523ac56

Co-authored-by: Craig Perkins <craig5008@gmail.com> Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com>

cwperks approved these changes Mar 13, 2026

View reviewed changes

RyanL1997 approved these changes Mar 13, 2026

View reviewed changes

cwperks merged commit 14adbeb into opensearch-project:main Mar 13, 2026
113 of 117 checks passed

cwperks mentioned this pull request Mar 13, 2026

Add ability to change default DLS parameter substitution behavior #1310

Closed

opensearch-ci-bot mentioned this pull request Apr 4, 2026

[AUTO] Add release notes for 3.6.0 #6063

Merged

rursprung reviewed Apr 23, 2026

View reviewed changes

This was referenced Apr 23, 2026

[FEATURE] support fallback values in DLS/FLS variables #6110

Open

simplify UserAttributes#findUnresolvedAttributes #6122

Merged

Conversation

ThyTran1402 commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Testing

Check List

Uh oh!

codecov Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nibix commented Mar 3, 2026

Uh oh!

ThyTran1402 commented Mar 3, 2026

Uh oh!

Uh oh!

ThyTran1402 commented Mar 11, 2026

Uh oh!

cwperks commented Mar 11, 2026

Uh oh!

Uh oh!

cwperks commented Mar 11, 2026

Uh oh!

cwperks commented Mar 11, 2026

Uh oh!

Uh oh!

Uh oh!

cwperks commented Mar 11, 2026

Uh oh!

ThyTran1402 commented Mar 12, 2026

Uh oh!

Uh oh!

ThyTran1402 commented Mar 13, 2026

Uh oh!

cwperks commented Mar 13, 2026

Uh oh!

Uh oh!

rursprung Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThyTran1402 Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rursprung Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ThyTran1402 commented Mar 1, 2026 •

edited

Loading

codecov Bot commented Mar 3, 2026 •

edited

Loading

rursprung Apr 23, 2026 •

edited

Loading

ThyTran1402 Apr 24, 2026 •

edited

Loading