fixed throw exception for DLS user attributes

CHANGELOG.md

@@ -18,6 +18,8 @@ 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

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,33 @@ 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 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(

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<String> findUnresolvedAttributes(String s) {
+        List<String> 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();
 

src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java

@@ -16,6 +16,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.apache.logging.log4j.util.Strings;
 
@@ -46,6 +48,8 @@
  * Instances of this class are managed by DlsFlsProcessedConfig.
  */
 public class DocumentPrivileges extends AbstractRuleBasedPrivileges<DocumentPrivileges.DlsQuery, DlsRestriction> {
+    private static final int MAX_ATTRIBUTES_IN_ERROR_MESSAGE = 10;
+
     private final NamedXContentRegistry xContentRegistry;
 
     public DocumentPrivileges(
@@ -176,8 +180,16 @@ static class Dynamic extends DlsQuery {
             RenderedDlsQuery evaluate(PrivilegesEvaluationContext context) throws PrivilegesEvaluationException {
                 String effectiveQueryString = UserAttributes.replaceProperties(this.queryString, context);
                 if (UserAttributes.needsAttributeSubstitution(effectiveQueryString)) {
+                    List<String> unresolved = UserAttributes.findUnresolvedAttributes(effectiveQueryString);
+                    Set<String> available = context.getUser().getCustomAttributesMap().keySet();
+                    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(
-                        "Invalid DLS query: " + effectiveQueryString,
+                        "DLS query references undefined user attributes: " + unresolved + ". Available attributes are: " + availableStr,
                         new OpenSearchSecurityException("User attribute substitution failed")
                     );
                 }

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
@@ -71,4 +73,34 @@ 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");
+    }
 }