Fix ACI grouped bind rule wrongly rejected when a value contains parentheses (#716)

Author: vharseko

opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java

@@ -13,6 +13,7 @@
  *
  * Copyright 2008 Sun Microsystems, Inc.
  * Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC
  */
 package org.opends.server.authorization.dseecompat;
 
@@ -133,11 +134,10 @@ private BindRule(BindRule left, BindRule right, EnumBooleanTypes booleanType) {
     }
 
     /*
-     * TODO Verify this method handles escaped parentheses by writing
-     * a unit test.
-     *
-     * It doesn't look like the decode() method handles the possibility of
-     * escaped parentheses in a bind rule.
+     * Parentheses embedded in a quoted bind rule expression (for example a DN
+     * value such as userdn="ldap:///cn=a(b),dc=example,dc=com") are treated as
+     * literal data and are not mistaken for grouping parentheses. See the
+     * quote-aware scan below and BindRuleParenTest for the covering cases.
      */
     /**
      * Decode an ACI bind rule string representation.
@@ -160,19 +160,27 @@ public static BindRule decode (String input) throws AciException {
           int currentPos;
           int numOpen = 0;
           int numClose = 0;
+          boolean inQuotes = false;
 
-          // Find the associated closed parenthesis
+          // Find the associated closed parenthesis. Parentheses that appear
+          // inside a quoted bind rule expression (e.g. a DN value containing
+          // '(' or ')') are literal data and must not be counted as grouping.
           for (currentPos = 0; currentPos < bindruleArray.length; currentPos++)
           {
-            if (bindruleArray[currentPos] == '(')
+            char currentChar = bindruleArray[currentPos];
+            if (currentChar == '"')
+            {
+              inQuotes = !inQuotes;
+            }
+            else if (!inQuotes && currentChar == '(')
             {
               numOpen++;
             }
-            else if (bindruleArray[currentPos] == ')')
+            else if (!inQuotes && currentChar == ')')
             {
               numClose++;
             }
-            if (numClose == numOpen)
+            if (!inQuotes && numClose == numOpen)
             {
               // We found the associated closed parenthesis the parenthesis are removed
               String bindruleStr1 = bindruleStr.substring(1, currentPos);

opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleParenTest.java

@@ -0,0 +1,99 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions Copyright [year] [name of copyright owner]".
+ *
+ * Portions Copyright 2026 3A Systems, LLC
+ */
+package org.opends.server.authorization.dseecompat;
+
+import static org.assertj.core.api.Assertions.*;
+
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.types.DirectoryException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+/**
+ * Verifies that {@link BindRule#decode(String)} treats parentheses embedded in a
+ * quoted bind rule expression (e.g. a DN value containing '(' or ')') as literal
+ * data instead of grouping parentheses. Before the quote-aware scan, a grouped
+ * bind rule whose value contained a parenthesis was wrongly rejected because the
+ * embedded parenthesis was counted when locating the matching close parenthesis.
+ * Also verifies the scan stays fail-closed: genuinely unbalanced grouping
+ * parentheses (outside quotes) are still rejected with an {@link AciException}.
+ */
+@SuppressWarnings("javadoc")
+public class BindRuleParenTest extends DirectoryServerTestCase
+{
+  @BeforeClass
+  public void setUp() throws Exception
+  {
+    TestCaseUtils.startFakeServer();
+  }
+
+  @AfterClass
+  public void tearDown() throws DirectoryException
+  {
+    TestCaseUtils.shutdownFakeServer();
+  }
+
+  @DataProvider(name = "acisWithParenInValue")
+  public Object[][] acisWithParenInValue()
+  {
+    return new Object[][] {
+      // simple (ungrouped) bind rule with a ')' / '(' in the DN value
+      { "(version 3.0; acl \"t\"; allow (search) userdn=\"ldap:///cn=a)b,dc=x\";)" },
+      { "(version 3.0; acl \"t\"; allow (search) userdn=\"ldap:///cn=a(b,dc=x\";)" },
+      // grouped bind rule with a ')' embedded in a value
+      { "(version 3.0; acl \"t\"; allow (search) "
+          + "(userdn=\"ldap:///cn=a)b,dc=x\" or userdn=\"ldap:///self\");)" },
+      // grouped bind rule with a '(' embedded in a value
+      { "(version 3.0; acl \"t\"; allow (search) "
+          + "(userdn=\"ldap:///cn=a(b,dc=x\" or userdn=\"ldap:///self\");)" },
+      // grouped bind rule with matched parentheses embedded in a value
+      { "(version 3.0; acl \"t\"; allow (search) "
+          + "(userdn=\"ldap:///cn=a(b),dc=x\" or userdn=\"ldap:///self\");)" },
+    };
+  }
+
+  @Test(dataProvider = "acisWithParenInValue")
+  public void decodesParenInsideQuotedValue(String aci) throws Exception
+  {
+    AciBody body = AciBody.decode(aci);
+    assertThat(body.getPermBindRulePairs()).hasSize(1);
+    assertThat(body.getPermBindRulePairs().get(0).getBindRule()).isNotNull();
+  }
+
+  @DataProvider(name = "malformedBindRules")
+  public Object[][] malformedBindRules()
+  {
+    return new Object[][] {
+      // grouped bind rule with no close parenthesis at all
+      { "(userdn=\"ldap:///self\"" },
+      // grouped bind rule with one extra (unmatched) open parenthesis
+      { "((userdn=\"ldap:///self\")" },
+      // grouped bind rule whose only ')' is embedded in a quoted value, so the
+      // group is genuinely unterminated: the quote-aware scan must not treat the
+      // embedded ')' as the closing parenthesis and silently accept the input.
+      { "(userdn=\"ldap:///cn=a)b,dc=x\" or userdn=\"ldap:///self\"" },
+    };
+  }
+
+  @Test(dataProvider = "malformedBindRules")
+  public void rejectsUnbalancedParensOutsideQuotes(String bindRule)
+  {
+    assertThatThrownBy(() -> BindRule.decode(bindRule)).isInstanceOf(AciException.class);
+  }
+}