From 99e2caadecab68a787d1e29672810a54d4e91792 Mon Sep 17 00:00:00 2001
From: Benjamin Eckstein <13351939+benjamineckstein@users.noreply.github.com>
Date: Thu, 30 Jan 2020 09:09:29 +0100
Subject: [PATCH] TSK-1024: Refactor and clean up LdapClient changes

---
 .../java/pro/taskana/ldap/LdapClient.java     | 42 +++++++++++--------
 .../java/pro/taskana/ldap/LdapClientTest.java | 12 +++++-
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/ldap/LdapClient.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/ldap/LdapClient.java
index 409a582dc..7728b5720 100644
--- a/rest/taskana-rest-spring/src/main/java/pro/taskana/ldap/LdapClient.java
+++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/ldap/LdapClient.java
@@ -73,6 +73,14 @@ public class LdapClient {
 
   private String message;
 
+  /**
+   * Search LDAP for matching users or groups.
+   *
+   * @param name lookup string for names or groups
+   * @return a list of AccessIdResources sorted by AccessId and limited to
+   *     maxNumberOfReturnedAccessIds
+   * @throws InvalidArgumentException if input is shorter than minSearchForLength
+   */
   public List<AccessIdResource> searchUsersAndGroups(final String name)
       throws InvalidArgumentException {
     LOGGER.debug("entry to searchUsersAndGroups(name = {})", name);
@@ -159,19 +167,12 @@ public class LdapClient {
     }
     andFilter.and(orFilter);
 
-    String[] groupAttributesToReturn;
-    if (CN.equals(groupNameAttribute)) {
-      groupAttributesToReturn = new String[] {CN};
-    } else {
-      groupAttributesToReturn = new String[] {getGroupNameAttribute(), CN};
-    }
-
     final List<AccessIdResource> accessIds =
         ldapTemplate.search(
             getGroupSearchBase(),
             andFilter.encode(),
             SearchControls.SUBTREE_SCOPE,
-            groupAttributesToReturn,
+            getLookUpGoupAttributesToReturn(),
             new GroupContextMapper());
     LOGGER.debug(
         "Exit from searchGroupsByName. Retrieved the following groups: {}",
@@ -189,22 +190,29 @@ public class LdapClient {
     // given DN.
     // https://stackoverflow.com/questions/55285743/spring-ldaptemplate-how-to-lookup-fully-qualified-dn-with-configured-base-dn
     // Therefore we have to remove the base name from the dn before performing the lookup
-    // (?i) --> case insensitive replacement
-    String nameWithoutBaseDn = name.replaceAll("(?i)" + Pattern.quote("," + baseDn), "");
+    String nameWithoutBaseDn = getNameWithoutBaseDn(name);
     LOGGER.debug(
         "Removes baseDN {} from given DN. New DN to be used: {}", baseDn, nameWithoutBaseDn);
-    String[] groupAttributesToReturn;
-    if (CN.equals(groupNameAttribute)) {
-      groupAttributesToReturn = new String[] {CN};
-    } else {
-      groupAttributesToReturn = new String[] {getGroupNameAttribute(), CN};
-    }
     final AccessIdResource accessId =
-        ldapTemplate.lookup(nameWithoutBaseDn, groupAttributesToReturn, new GroupContextMapper());
+        ldapTemplate.lookup(
+            nameWithoutBaseDn, getLookUpGoupAttributesToReturn(), new GroupContextMapper());
     LOGGER.debug("Exit from searchGroupByDn. Retrieved the following group: {}", accessId);
     return accessId;
   }
 
+  String getNameWithoutBaseDn(String name) {
+    // (?i) --> case insensitive replacement
+    return name.replaceAll("(?i)" + Pattern.quote("," + baseDn), "");
+  }
+
+  String[] getLookUpGoupAttributesToReturn() {
+    if (CN.equals(groupNameAttribute)) {
+      return new String[] {CN};
+    } else {
+      return new String[] {getGroupNameAttribute(), CN};
+    }
+  }
+
   public List<AccessIdResource> searchGroupsofUsersIsMember(final String name)
       throws InvalidArgumentException {
     LOGGER.debug("entry to searchGroupsofUsersIsMember(name = {}).", name);
diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/ldap/LdapClientTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/ldap/LdapClientTest.java
index 7060a5d03..c182b5d8a 100644
--- a/rest/taskana-rest-spring/src/test/java/pro/taskana/ldap/LdapClientTest.java
+++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/ldap/LdapClientTest.java
@@ -1,5 +1,6 @@
 package pro.taskana.ldap;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.verify;
@@ -24,7 +25,7 @@ class LdapClientTest {
   @InjectMocks LdapClient cut;
 
   @Test
-  void testLdap() {
+  void testLdap_searchGroupByDn() {
 
     setUpEnvMock();
     cut.init();
@@ -36,6 +37,15 @@ class LdapClientTest {
             eq("cn=developersgroup,ou=groups"), any(), any(LdapClient.GroupContextMapper.class));
   }
 
+  @Test
+  void testLdap_getNameWithoutBaseDn() {
+
+    setUpEnvMock();
+    cut.init();
+    assertThat(cut.getNameWithoutBaseDn("cn=developersgroup,ou=groups,o=taskanatest"))
+        .isEqualTo("cn=developersgroup,ou=groups");
+  }
+
   private void setUpEnvMock() {
 
     Stream.of(