Bugfix/ldap search query

src/main/java/org/elasticsearch/plugin/readonlyrest/acl/definitions/ldaps/unboundid/UnboundidBaseLdapClient.java

@@ -17,6 +17,7 @@
 package org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps.unboundid;
 
 import com.google.common.collect.Lists;
+import com.unboundid.ldap.sdk.Filter;
 import com.unboundid.ldap.sdk.LDAPException;
 import com.unboundid.ldap.sdk.SearchRequest;
 import com.unboundid.ldap.sdk.SearchResultEntry;
@@ -59,7 +60,7 @@ public CompletableFuture<Optional<LdapUser>> userById(String userId) {
                   new UnboundidSearchResultListener(searchUser),
                   userSearchFilterConfig.getSearchUserBaseDN(),
                   SearchScope.SUB,
-                  String.format("(%s=%s)", userSearchFilterConfig.getUidAttribute(), userId)
+                  String.format("(%s=%s)", userSearchFilterConfig.getUidAttribute(), Filter.encodeValue(userId))
               )),
           requestTimeout.toMillis()
       );

src/main/java/org/elasticsearch/plugin/readonlyrest/acl/definitions/ldaps/unboundid/UnboundidGroupsProviderLdapClient.java

@@ -18,6 +18,7 @@
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
+import com.unboundid.ldap.sdk.Filter;
 import com.unboundid.ldap.sdk.LDAPException;
 import com.unboundid.ldap.sdk.SearchRequest;
 import com.unboundid.ldap.sdk.SearchResultEntry;
@@ -72,7 +73,10 @@ public CompletableFuture<Set<LdapGroup>> userGroups(LdapUser user) {
                   new UnboundidSearchResultListener(searchGroups),
                   userGroupsSearchFilterConfig.getSearchGroupBaseDN(),
                   SearchScope.SUB,
-                  String.format("(&(cn=*)(%s=%s))", userGroupsSearchFilterConfig.getUniqueMemberAttribute(), user.getDN())
+                  String.format(
+                      "(&(cn=*)(%s=%s))", userGroupsSearchFilterConfig.getUniqueMemberAttribute(),
+                      Filter.encodeValue(user.getDN())
+                  )
               )),
           requestTimeout.toMillis()
       );

src/test/java/org/elasticsearch/plugin/readonlyrest/acl/definitions/ldaps/UnboundidAuthenticationLdapClientTests.java

@@ -17,8 +17,10 @@
 
 package org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps;
 
+import com.google.common.collect.Sets;
 import org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps.unboundid.ConnectionConfig;
-import org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps.unboundid.UnboundidAuthenticationLdapClient;
+import org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps.unboundid.UnboundidGroupsProviderLdapClient;
+import org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps.unboundid.UserGroupsSearchFilterConfig;
 import org.elasticsearch.plugin.readonlyrest.acl.definitions.ldaps.unboundid.UserSearchFilterConfig;
 import org.elasticsearch.plugin.readonlyrest.utils.containers.LdapContainer;
 import org.elasticsearch.plugin.readonlyrest.utils.esdependent.MockedESContext;
@@ -28,14 +30,15 @@
 
 import java.time.Duration;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 
 public class UnboundidAuthenticationLdapClientTests {
 
   @ClassRule
   public static LdapContainer ldapContainer = LdapContainer.create("/test_example.ldif");
 
-  private UnboundidAuthenticationLdapClient client = new UnboundidAuthenticationLdapClient(
+  private UnboundidGroupsProviderLdapClient client = new UnboundidGroupsProviderLdapClient(
       new ConnectionConfig.Builder(ldapContainer.getLdapHost())
           .setPort(ldapContainer.getLdapPort())
           .setPoolSize(10)
@@ -45,6 +48,7 @@ public class UnboundidAuthenticationLdapClientTests {
           .setTrustAllCerts(false)
           .build(),
       new UserSearchFilterConfig.Builder("ou=People,dc=example,dc=com").build(),
+      new UserGroupsSearchFilterConfig.Builder("ou=Groups,dc=example,dc=com").build(),
       Optional.of(ldapContainer.getSearchingUserConfig()),
       MockedESContext.INSTANCE
   );
@@ -55,7 +59,7 @@ public void testAuthenticationSuccess() throws Exception {
         new LdapCredentials("cartman", "user2")
     );
     Optional<LdapUser> user = userF.get();
-    Assert.assertEquals(user.isPresent(), true);
+    Assert.assertEquals(true, user.isPresent());
   }
 
   @Test
@@ -64,7 +68,7 @@ public void testAuthenticationErrorDueToInvalidPassword() throws Exception {
         new LdapCredentials("cartman", "wrongpassword")
     );
     Optional<LdapUser> user = userF.get();
-    Assert.assertEquals(user.isPresent(), false);
+    Assert.assertEquals(false, user.isPresent());
   }
 
   @Test
@@ -73,6 +77,16 @@ public void testAuthenticationErrorDueToUnknownUser() throws Exception {
         new LdapCredentials("nonexistent", "whatever")
     );
     Optional<LdapUser> user = userF.get();
-    Assert.assertEquals(user.isPresent(), false);
+    Assert.assertEquals(false, user.isPresent());
+  }
+
+  @Test
+  public void testAuthenticationSuccessAndCNIsProperlyEscaped() throws Exception {
+    CompletableFuture<Set<LdapGroup>> groupsF = client.authenticate(new LdapCredentials("viktor", "user2"))
+        .thenCompose(ldapUser ->
+            ldapUser.map(user -> client.userGroups(user))
+                .orElse(CompletableFuture.completedFuture(Sets.newHashSet()))
+        );
+    Assert.assertEquals(false, groupsF.get().isEmpty());
   }
 }

src/test/resources/test_example.ldif

@@ -35,6 +35,16 @@ sn: Bong
 uid: bong
 userPassword:: e1NIQX1zOXFuZTB3RXFWVWJoNEhRTVpIK0NZOHlYbWM9
 
+dn: cn=Viktor Navorski (Tom Hanks),ou=People,dc=example,dc=com
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Viktor Navorski (Tom Hanks)
+sn: Navorski
+uid: viktor
+userPassword:: e1NNRDV9czdnM0NVekVCMGQxMm5CM0N3VGFrQmp3K0VGMTE3cFg=
+
 dn: ou=Groups,dc=example,dc=com
 objectClass: top
 objectClass: organizationalUnit
@@ -60,3 +70,4 @@ cn: group3
 uniqueMember: cn=Chanandler Bong,ou=People,dc=example,dc=com
 uniqueMember: cn=Eric Cartman,ou=People,dc=example,dc=com
 uniqueMember: cn=Morgan Freeman,ou=People,dc=example,dc=com
+uniqueMember: cn=Viktor Navorski (Tom Hanks),ou=People,dc=example,dc=com