From 5c77a2d212f1a87c45b1bf3ef99e5687500a103d Mon Sep 17 00:00:00 2001 From: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> Date: Sun, 1 Mar 2026 11:38:03 -0500 Subject: [PATCH 1/6] fixed throw exception for DLS user attributes Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> --- .../dlsfls/DocumentPrivilegesTest.java | 13 +++++++++ .../security/privileges/UserAttributes.java | 20 +++++++++++++ .../privileges/dlsfls/DocumentPrivileges.java | 6 +++- .../privileges/UserAttributesUnitTest.java | 28 +++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java index 7182b22ed6..4926814225 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java @@ -64,6 +64,7 @@ import org.opensearch.test.framework.TestSecurityConfig; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.opensearch.security.util.MockIndexMetadataBuilder.dataStreams; import static org.opensearch.security.util.MockIndexMetadataBuilder.indices; import static org.junit.Assert.assertEquals; @@ -1175,6 +1176,18 @@ public void invalidTemplatedQuery() throws Exception { .evaluate(MockPrivilegeEvaluationContextBuilder.ctx().get()); } + @Test + public void invalidTemplatedQuery_errorMessageIdentifiesUndefinedAttribute() throws Exception { + try { + DocumentPrivileges.DlsQuery.create("{\"terms\":{\"arr\":[${attr.jwt.array}]}}", xContentRegistry) + .evaluate(MockPrivilegeEvaluationContextBuilder.ctx().attr("attr.jwt.other", "x").get()); + fail("Expected PrivilegesEvaluationException"); + } catch (PrivilegesEvaluationException e) { + assertThat(e.getMessage(), containsString("attr.jwt.array")); + assertThat(e.getMessage(), containsString("attr.jwt.other")); + } + } + @Test public void equals() throws Exception { DocumentPrivileges.DlsQuery query1a = DocumentPrivileges.DlsQuery.create( diff --git a/src/main/java/org/opensearch/security/privileges/UserAttributes.java b/src/main/java/org/opensearch/security/privileges/UserAttributes.java index f1a8acf148..14655ea8f5 100644 --- a/src/main/java/org/opensearch/security/privileges/UserAttributes.java +++ b/src/main/java/org/opensearch/security/privileges/UserAttributes.java @@ -10,8 +10,13 @@ */ package org.opensearch.security.privileges; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import com.google.common.base.Joiner; import com.google.common.collect.Iterables; @@ -25,10 +30,25 @@ * This code was moved over from ConfigModelV7. */ public class UserAttributes { + private static final Pattern UNRESOLVED_ATTRIBUTE_PATTERN = Pattern.compile("\\$\\{([^}]+)\\}"); + public static boolean needsAttributeSubstitution(String patternString) { return patternString.contains("${"); } + /** + * Returns the names of all unresolved ${...} attribute references remaining + * in {@code s} after substitution has been performed. + */ + public static List findUnresolvedAttributes(String s) { + List result = new ArrayList<>(); + Matcher matcher = UNRESOLVED_ATTRIBUTE_PATTERN.matcher(s); + while (matcher.find()) { + result.add(matcher.group(1)); + } + return Collections.unmodifiableList(result); + } + public static String replaceProperties(String orig, PrivilegesEvaluationContext context) { User user = context.getUser(); diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java index bb4563517e..0c09b7c211 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import org.apache.logging.log4j.util.Strings; @@ -176,8 +177,11 @@ static class Dynamic extends DlsQuery { RenderedDlsQuery evaluate(PrivilegesEvaluationContext context) throws PrivilegesEvaluationException { String effectiveQueryString = UserAttributes.replaceProperties(this.queryString, context); if (UserAttributes.needsAttributeSubstitution(effectiveQueryString)) { + List unresolved = UserAttributes.findUnresolvedAttributes(effectiveQueryString); + Set available = context.getUser().getCustomAttributesMap().keySet(); throw new PrivilegesEvaluationException( - "Invalid DLS query: " + effectiveQueryString, + "DLS query references undefined user attributes: " + unresolved + + ". Available attributes are: " + available, new OpenSearchSecurityException("User attribute substitution failed") ); } diff --git a/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java b/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java index 05da8d5419..facc23cafa 100644 --- a/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java +++ b/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java @@ -71,4 +71,32 @@ public void testReplaceProperties() { """; assertEquals(expectedString, UserAttributes.replaceProperties(stringWithPlaceholders, ctx)); } + + @Test + public void testFindUnresolvedAttributes_noTokens() { + assertTrue(UserAttributes.findUnresolvedAttributes("{\"term\": {\"dept\": \"value\"}}").isEmpty()); + } + + @Test + public void testFindUnresolvedAttributes_singleToken() { + assertEquals( + List.of("attr.jwt.array"), + UserAttributes.findUnresolvedAttributes("{\"terms\":{\"arr\":[${attr.jwt.array}]}}") + ); + } + + @Test + public void testFindUnresolvedAttributes_multipleTokens() { + assertEquals( + List.of("attr.jwt.dept", "attr.proxy.region"), + UserAttributes.findUnresolvedAttributes( + "{\"bool\":{\"must\":[{\"term\":{\"dept\":\"${attr.jwt.dept}\"}},{\"term\":{\"region\":\"${attr.proxy.region}\"}}]}}" + ) + ); + } + + @Test(expected = UnsupportedOperationException.class) + public void testFindUnresolvedAttributes_returnsUnmodifiableList() { + UserAttributes.findUnresolvedAttributes("${attr.foo}").add("extra"); + } } From e0373eb93fc882e54e059d6697b243f27ee77215 Mon Sep 17 00:00:00 2001 From: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> Date: Tue, 3 Mar 2026 13:20:25 -0500 Subject: [PATCH 2/6] updated changelog Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1828035d85..a3f6c753ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix audit log writing errors for rollover-enabled alias indices ([#5878](https://github.com/opensearch-project/security/pull/5878) - Fix the issue of unprocessed X-Request-Id ([#5954](https://github.com/opensearch-project/security/pull/5954)) +- Improve DLS error message to identify undefined user attributes when query substitution fails ([#5975](https://github.com/opensearch-project/security/pull/5975)) + +- Fix the issue of unprocessed X-Request-Id ([#5976](https://github.com/opensearch-project/security/pull/5976)) ### Refactoring ### Maintenance From 4377cfb3f41c7794982c733f5b8a60edc9dd1d2e Mon Sep 17 00:00:00 2001 From: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> Date: Fri, 6 Mar 2026 10:59:26 -0500 Subject: [PATCH 3/6] set the limit for amount of chars Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> --- .../security/privileges/dlsfls/DocumentPrivileges.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java index 0c09b7c211..7e34b6423f 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java @@ -17,6 +17,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import org.apache.logging.log4j.util.Strings; @@ -179,9 +180,12 @@ RenderedDlsQuery evaluate(PrivilegesEvaluationContext context) throws Privileges if (UserAttributes.needsAttributeSubstitution(effectiveQueryString)) { List unresolved = UserAttributes.findUnresolvedAttributes(effectiveQueryString); Set available = context.getUser().getCustomAttributesMap().keySet(); + String availableStr = available.size() > 10 + ? available.stream().limit(10).collect(Collectors.joining(", ")) + " ... and " + (available.size() - 10) + " more" + : String.join(", ", available); throw new PrivilegesEvaluationException( "DLS query references undefined user attributes: " + unresolved - + ". Available attributes are: " + available, + + ". Available attributes are: " + availableStr, new OpenSearchSecurityException("User attribute substitution failed") ); } From 42dc139a0309d4018407485ae4ec81fe8f4cb066 Mon Sep 17 00:00:00 2001 From: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> Date: Wed, 11 Mar 2026 13:27:23 -0400 Subject: [PATCH 4/6] fixed hard coding limit and use text blocks Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> --- CHANGELOG.md | 1 - .../privileges/dlsfls/DocumentPrivileges.java | 12 +++++--- .../privileges/UserAttributesUnitTest.java | 30 +++++++++++-------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3f6c753ea..a8f6b7ec04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix the issue of unprocessed X-Request-Id ([#5954](https://github.com/opensearch-project/security/pull/5954)) - Improve DLS error message to identify undefined user attributes when query substitution fails ([#5975](https://github.com/opensearch-project/security/pull/5975)) - - Fix the issue of unprocessed X-Request-Id ([#5976](https://github.com/opensearch-project/security/pull/5976)) ### Refactoring diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java index 7e34b6423f..37548f3c8a 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java @@ -48,6 +48,8 @@ * Instances of this class are managed by DlsFlsProcessedConfig. */ public class DocumentPrivileges extends AbstractRuleBasedPrivileges { + private static final int MAX_ATTRIBUTES_IN_ERROR_MESSAGE = 10; + private final NamedXContentRegistry xContentRegistry; public DocumentPrivileges( @@ -180,12 +182,14 @@ RenderedDlsQuery evaluate(PrivilegesEvaluationContext context) throws Privileges if (UserAttributes.needsAttributeSubstitution(effectiveQueryString)) { List unresolved = UserAttributes.findUnresolvedAttributes(effectiveQueryString); Set available = context.getUser().getCustomAttributesMap().keySet(); - String availableStr = available.size() > 10 - ? available.stream().limit(10).collect(Collectors.joining(", ")) + " ... and " + (available.size() - 10) + " more" + String availableStr = available.size() > MAX_ATTRIBUTES_IN_ERROR_MESSAGE + ? available.stream().limit(MAX_ATTRIBUTES_IN_ERROR_MESSAGE).collect(Collectors.joining(", ")) + + " ... and " + + (available.size() - MAX_ATTRIBUTES_IN_ERROR_MESSAGE) + + " more" : String.join(", ", available); throw new PrivilegesEvaluationException( - "DLS query references undefined user attributes: " + unresolved - + ". Available attributes are: " + availableStr, + "DLS query references undefined user attributes: " + unresolved + ". Available attributes are: " + availableStr, new OpenSearchSecurityException("User attribute substitution failed") ); } diff --git a/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java b/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java index facc23cafa..665fcc3c3f 100644 --- a/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java +++ b/src/test/java/org/opensearch/security/privileges/UserAttributesUnitTest.java @@ -25,9 +25,11 @@ public class UserAttributesUnitTest { @Test public void testNeedsAttributeSubstitution() { - assertTrue(UserAttributes.needsAttributeSubstitution("{\"foo\": \"${user.name}}\"")); + assertTrue(UserAttributes.needsAttributeSubstitution(""" + {"foo": "${user.name}}"}""")); assertTrue(UserAttributes.needsAttributeSubstitution("${attr1.proxy.foo}")); - assertFalse(UserAttributes.needsAttributeSubstitution("{\"foo\": \"bar\"}")); + assertFalse(UserAttributes.needsAttributeSubstitution(""" + {"foo": "bar"}""")); } @Test @@ -74,25 +76,27 @@ public void testReplaceProperties() { @Test public void testFindUnresolvedAttributes_noTokens() { - assertTrue(UserAttributes.findUnresolvedAttributes("{\"term\": {\"dept\": \"value\"}}").isEmpty()); + assertTrue(UserAttributes.findUnresolvedAttributes(""" + {"term": {"dept": "value"}}""").isEmpty()); } @Test public void testFindUnresolvedAttributes_singleToken() { - assertEquals( - List.of("attr.jwt.array"), - UserAttributes.findUnresolvedAttributes("{\"terms\":{\"arr\":[${attr.jwt.array}]}}") - ); + assertEquals(List.of("attr.jwt.array"), UserAttributes.findUnresolvedAttributes(""" + {"terms": {"arr": [${attr.jwt.array}]}}""")); } @Test public void testFindUnresolvedAttributes_multipleTokens() { - assertEquals( - List.of("attr.jwt.dept", "attr.proxy.region"), - UserAttributes.findUnresolvedAttributes( - "{\"bool\":{\"must\":[{\"term\":{\"dept\":\"${attr.jwt.dept}\"}},{\"term\":{\"region\":\"${attr.proxy.region}\"}}]}}" - ) - ); + assertEquals(List.of("attr.jwt.dept", "attr.proxy.region"), UserAttributes.findUnresolvedAttributes(""" + { + "bool": { + "must": [ + {"term": {"dept": "${attr.jwt.dept}"}}, + {"term": {"region": "${attr.proxy.region}"}} + ] + } + }""")); } @Test(expected = UnsupportedOperationException.class) From 5a47c7bcc2a061ea79532b10d359928f8cbfc7d4 Mon Sep 17 00:00:00 2001 From: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> Date: Thu, 12 Mar 2026 14:31:04 -0400 Subject: [PATCH 5/6] added unit test for more than 10 attributes Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> --- .../privileges/dlsfls/DocumentPrivilegesTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java index 4926814225..87a8ca60d8 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DocumentPrivilegesTest.java @@ -1188,6 +1188,21 @@ public void invalidTemplatedQuery_errorMessageIdentifiesUndefinedAttribute() thr } } + @Test + public void invalidTemplatedQuery_errorMessageTruncatesWhenMoreThanTenAvailableAttributes() throws Exception { + MockPrivilegeEvaluationContextBuilder ctx = MockPrivilegeEvaluationContextBuilder.ctx(); + for (int i = 1; i <= 12; i++) { + ctx.attr("attr.jwt.attr" + i, "v" + i); + } + try { + DocumentPrivileges.DlsQuery.create("{\"term\":{\"dept\":\"${attr.jwt.missing}\"}}", xContentRegistry).evaluate(ctx.get()); + fail("Expected PrivilegesEvaluationException"); + } catch (PrivilegesEvaluationException e) { + assertThat(e.getMessage(), containsString("attr.jwt.missing")); + assertThat(e.getMessage(), containsString("... and 2 more")); + } + } + @Test public void equals() throws Exception { DocumentPrivileges.DlsQuery query1a = DocumentPrivileges.DlsQuery.create( From 523ac56343ea17758044c3d885e9a3c43170fcee Mon Sep 17 00:00:00 2001 From: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> Date: Thu, 12 Mar 2026 14:55:43 -0400 Subject: [PATCH 6/6] Update CHANGELOG.md Co-authored-by: Craig Perkins Signed-off-by: Thy Tran <58045538+ThyTran1402@users.noreply.github.com> --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8f6b7ec04..8991e7f74c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix the issue of unprocessed X-Request-Id ([#5954](https://github.com/opensearch-project/security/pull/5954)) - Improve DLS error message to identify undefined user attributes when query substitution fails ([#5975](https://github.com/opensearch-project/security/pull/5975)) -- Fix the issue of unprocessed X-Request-Id ([#5976](https://github.com/opensearch-project/security/pull/5976)) ### Refactoring ### Maintenance