[MINOR] Address review feedback on LDAP escaping changes

- Apply RFC 4515 escaping to the matching-rule-in-chain filter site
  (groupObjectClass, memberAttribute, userDn) — previously this branch
  used String.format with no escape on values that may include parts of
  the principal once group membership is resolved through the chain.
- Preserve the bare-placeholder DN template behaviour: when
  userDnTemplate is exactly "{0}", treat the principal as a full DN
  supplied verbatim rather than running RFC 4514 escape on it. Escaping
  collapsed comma-separated DNs into a single RDN value and broke binds
  for deployments that rely on full-DN principals.
- Stop applying RFC 4514 escape inside setUserSearchFilter and
  setGroupSearchFilter. The values are operator-supplied filter
  templates; the placeholder substitution itself is the only place that
  needs escaping, and that already happens at expandFilterTemplate time.
- Update LdapRealmTest expectations to reflect the verbatim template
  storage and the bare-placeholder passthrough.
- Switch the second-call-site coverage in
  ActiveDirectoryGroupRealmFilterInjectionTest to actually invoke
  getRoleNamesForUser via reflection rather than re-running
  searchForUserName.
- Drop the unused assertFalse imports flagged by Checkstyle and replace
  the raw NUL byte in LdapRealmDnInjectionTest with the Java escape
  sequence to keep the source file plain text.
- Tighten the inline comment on the bare-placeholder branch and remove
  '=' from the expandDnTemplate Javadoc since RFC 4514 does not require
  escaping '=' inside an attribute value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jongyoul

zeppelin-server/src/main/java/org/apache/zeppelin/realm/LdapRealm.java

@@ -363,7 +363,10 @@ protected Set<String> rolesFor(PrincipalCollection principals, String userNameIn
           searchResultEnum = ldapCtx.search(
               getGroupSearchBase(),
               String.format(
-                  MATCHING_RULE_IN_CHAIN_FORMAT, groupObjectClass, memberAttribute, userDn),
+                  MATCHING_RULE_IN_CHAIN_FORMAT,
+                  LdapFilterEncoder.escapeFilterValue(groupObjectClass),
+                  LdapFilterEncoder.escapeFilterValue(memberAttribute),
+                  LdapFilterEncoder.escapeFilterValue(userDn)),
               searchControls);
           while (searchResultEnum != null && searchResultEnum.hasMore()) {
             // searchResults contains all the groups in search scope
@@ -774,15 +777,15 @@ public String getUserSearchFilter() {
   }
 
   public void setUserSearchFilter(final String filter) {
-    this.userSearchFilter = (filter == null ? null : escapeAttributeValue(filter.trim()));
+    this.userSearchFilter = (filter == null ? null : filter.trim());
   }
 
   public String getGroupSearchFilter() {
     return groupSearchFilter;
   }
 
   public void setGroupSearchFilter(final String filter) {
-    this.groupSearchFilter = (filter == null ? null : escapeAttributeValue(filter.trim()));
+    this.groupSearchFilter = (filter == null ? null : filter.trim());
   }
 
   public boolean getUserLowerCase() {
@@ -887,7 +890,13 @@ protected String getUserDn(final String principal) throws IllegalArgumentExcepti
     // If not searching use the userDnTemplate and return.
     if ((userSearchBase == null || userSearchBase.isEmpty()) || (userSearchAttributeName == null
         && userSearchFilter == null && !"object".equalsIgnoreCase(userSearchScope))) {
-      userDn = expandDnTemplate(userDnTemplate, matchedPrincipal);
+      // Bare-placeholder template: principal is already a full DN; escaping
+      // its commas would collapse the RDNs and break binds.
+      if (MEMBER_SUBSTITUTION_TOKEN.equals(userDnTemplate)) {
+        userDn = matchedPrincipal;
+      } else {
+        userDn = expandDnTemplate(userDnTemplate, matchedPrincipal);
+      }
       LOGGER.debug("LDAP UserDN and Principal: {},{}", userDn, principal);
       return userDn;
     }
@@ -1045,8 +1054,8 @@ protected static final String expandFilterTemplate(final String template, final
    * (e.g. {@code userDnTemplate}, {@code userSearchBase}). The {@code input}
    * value is RFC 4514 escaped via the existing {@link #escapeAttributeValue}
    * before substitution so that user-controlled DN metacharacters such as
-   * {@code ,}, {@code =}, {@code +} or leading {@code #} cannot break out of
-   * their RDN or inject additional RDNs.
+   * {@code ,}, {@code +} or leading {@code #} cannot break out of their RDN
+   * or inject additional RDNs.
    */
   protected final String expandDnTemplate(final String template, final String input) {
     String escaped = escapeAttributeValue(input);

zeppelin-server/src/test/java/org/apache/zeppelin/realm/ActiveDirectoryGroupRealmFilterInjectionTest.java

@@ -17,7 +17,6 @@
 package org.apache.zeppelin.realm;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -79,30 +78,23 @@ void searchForUserName_escapes_user_payload(String payload) throws Exception {
       "admin)(cn=a*",
       ")(mail=*@corp.com"
   })
-  void doGetAuthorizationInfo_path_escapes_user_payload(String payload)
-      throws Exception {
-    // Drive getRoleNamesForUser indirectly via searchForUserName parameters
-    // since getRoleNamesForUser is package-private; this exercises the same
-    // String.format filter site at L304 with userPrincipalName.
+  void getRoleNamesForUser_escapes_user_payload(String payload) throws Exception {
+    // Exercise the second String.format filter site directly. The method is
+    // private, so use reflection to invoke it.
     LdapContext ctx = mock(LdapContext.class);
     NamingEnumeration<SearchResult> empty = mock(NamingEnumeration.class);
     when(empty.hasMoreElements()).thenReturn(false);
     when(ctx.search(anyString(), anyString(), any(), any(SearchControls.class)))
         .thenReturn(empty);
 
-    ActiveDirectoryGroupRealm realm = new ActiveDirectoryGroupRealm() {
-      // Expose the package-private method through a test-only hook.
-      java.util.Set<String> testGetRoleNamesForUser(String username, LdapContext c)
-          throws javax.naming.NamingException {
-        // Mirror the call chain in queryForAuthorizationInfo
-        return new java.util.LinkedHashSet<>();
-      }
-    };
+    ActiveDirectoryGroupRealm realm = new ActiveDirectoryGroupRealm();
     realm.setSearchBase("dc=example,dc=com");
 
-    // Use searchForUserName as proxy — the same escape utility is applied at
-    // both filter sites and the assertion holds for either one.
-    realm.searchForUserName(payload, ctx, 100);
+    java.lang.reflect.Method method =
+        ActiveDirectoryGroupRealm.class.getDeclaredMethod(
+            "getRoleNamesForUser", String.class, LdapContext.class);
+    method.setAccessible(true);
+    method.invoke(realm, payload, ctx);
 
     ArgumentCaptor<String> filterCap = ArgumentCaptor.forClass(String.class);
     verify(ctx).search(anyString(), filterCap.capture(), any(),

zeppelin-server/src/test/java/org/apache/zeppelin/realm/LdapRealmDnInjectionTest.java

@@ -17,7 +17,6 @@
 package org.apache.zeppelin.realm;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.junit.jupiter.api.Test;

zeppelin-server/src/test/java/org/apache/zeppelin/realm/LdapRealmFilterInjectionTest.java

@@ -47,10 +47,11 @@ class LdapRealmTest {
   void testGetUserDn() {
     LdapRealm realm = new LdapRealm();
 
-    // without a user search filter — trailing space is RFC 4514-escaped (\\20)
-    // because getUserDn expands via expandDnTemplate which calls escapeAttributeValue.
+    // without a user search filter — when userDnTemplate is the bare
+    // placeholder, the principal is treated as a full DN supplied verbatim
+    // (no escape) so trailing-space inputs are preserved unchanged.
     realm.setUserSearchFilter(null);
-    assertEquals("foo\\20", realm.getUserDn("foo "));
+    assertEquals("foo ", realm.getUserDn("foo "));
 
     // with a user search filter
     realm.setUserSearchFilter("memberUid={0}");
@@ -127,10 +128,13 @@ void testFilterEscaping() {
     assertEquals("foo\\2B", realm.escapeAttributeValue("foo+"));
     assertEquals("foo\\5C", realm.escapeAttributeValue("foo\\"));
     assertEquals("foo\\00", realm.escapeAttributeValue("foo\u0000"));
+    // setUserSearchFilter / setGroupSearchFilter store the operator-supplied
+    // template verbatim; user-controlled values are escaped at substitution
+    // time by expandFilterTemplate, not at config time.
     realm.setUserSearchFilter("uid=<{0}>");
-    assertEquals("uid=\\3C{0}\\3E", realm.getUserSearchFilter());
+    assertEquals("uid=<{0}>", realm.getUserSearchFilter());
     realm.setUserSearchFilter("gid=\\{0}\\");
-    assertEquals("gid=\\5C{0}\\5C", realm.getUserSearchFilter());
+    assertEquals("gid=\\{0}\\", realm.getUserSearchFilter());
   }
 
   private NamingEnumeration<SearchResult> enumerationOf(BasicAttributes... attrs) {