DaanHoogland closed pull request #2517: manual mapped ldap fix
URL: https://github.com/apache/cloudstack/pull/2517
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
index e844df57c1c..a8f14dfcd54 100644
--- 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
+++ 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
@@ -59,7 +59,7 @@
         return users;
     }
 
-    private String generateADGroupSearchFilter(String groupName, Long 
domainId) {
+    String generateADGroupSearchFilter(String groupName, Long domainId) {
         final StringBuilder userObjectFilter = new StringBuilder();
         userObjectFilter.append("(objectClass=");
         userObjectFilter.append(_ldapConfiguration.getUserObject(domainId));
diff --git 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
index cd4ed3d5cea..517c7185e29 100644
--- 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
+++ 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
@@ -215,7 +215,7 @@ private void processLdapUser(String password, Long 
domainId, UserAccount user, P
      * @param user cloudstack user object
      * @return false if either user object does not exist or authenitication 
fails
      */
-    private Pair<Boolean, ActionOnFailedAuthentication> authenticate(String 
username, String password, Long domainId, UserAccount user) {
+    Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, 
String password, Long domainId, UserAccount user) {
         boolean result = false;
 
         if(user != null ) {
@@ -231,8 +231,8 @@ private void processLdapUser(String password, Long 
domainId, UserAccount user, P
             }
         }
         return (!result && user != null) ?
-                new Pair<Boolean, ActionOnFailedAuthentication>(false, 
ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT):
-                new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
+                new Pair<Boolean, ActionOnFailedAuthentication>(result, 
ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT):
+                new Pair<Boolean, ActionOnFailedAuthentication>(result, null);
     }
 
     private void enableUserInCloudStack(UserAccount user) {
diff --git 
a/plugins/user-authenticators/ldap/test/org/apache/cloudstack/api/command/LdapConfigurationChanger.java
 
b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/api/command/LdapConfigurationChanger.java
index 61aa959e81a..38f7c9ba3ed 100644
--- 
a/plugins/user-authenticators/ldap/test/org/apache/cloudstack/api/command/LdapConfigurationChanger.java
+++ 
b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/api/command/LdapConfigurationChanger.java
@@ -38,7 +38,7 @@ default void setHiddenField(Object target, final String name, 
final Object o) th
      * the first field found by this name in the class "klas" or any of it's 
superclasses except for {@code Object}. Implementers of this interface can 
decide to also return any field in implemented interfaces or in {@code Object}.
      *
      * @param name of the field to find
-     * @param klas class to gat a field by name "name" from
+     * @param klas class to get a field by name "name" from
      * @return a {@code Field} by the name "name"
      * @throws NoSuchFieldException
      */
diff --git 
a/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java
 
b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java
new file mode 100644
index 00000000000..c2fc7ee4b5e
--- /dev/null
+++ 
b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.ldap;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.naming.directory.SearchControls;
+import javax.naming.ldap.LdapContext;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ADLdapUserManagerImplTest {
+
+    ADLdapUserManagerImpl adLdapUserManager;
+
+    @Mock
+    LdapConfiguration ldapConfiguration;
+
+    @Before
+    public void init() throws Exception {
+        adLdapUserManager = new ADLdapUserManagerImpl();
+        adLdapUserManager._ldapConfiguration = ldapConfiguration;
+    }
+
+    @Test
+    public void testGenerateADSearchFilterWithNestedGroupsEnabled() {
+        when(ldapConfiguration.getUserObject(any())).thenReturn("user");
+        when(ldapConfiguration.getCommonNameAttribute()).thenReturn("CN");
+        
when(ldapConfiguration.getBaseDn(any())).thenReturn("DC=cloud,DC=citrix,DC=com");
+        when(ldapConfiguration.isNestedGroupsEnabled(any())).thenReturn(true);
+
+        String [] groups = {"dev", "dev-hyd"};
+        for (String group: groups) {
+            String result = 
adLdapUserManager.generateADGroupSearchFilter(group, 1L);
+            
assertTrue(("(&(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=" + 
group + ",DC=cloud,DC=citrix,DC=com))").equals(result));
+        }
+
+    }
+
+    @Test
+    public void testGenerateADSearchFilterWithNestedGroupsDisabled() {
+        when(ldapConfiguration.getUserObject(any())).thenReturn("user");
+        when(ldapConfiguration.getCommonNameAttribute()).thenReturn("CN");
+        
when(ldapConfiguration.getBaseDn(any())).thenReturn("DC=cloud,DC=citrix,DC=com");
+        when(ldapConfiguration.isNestedGroupsEnabled(any())).thenReturn(false);
+
+        String [] groups = {"dev", "dev-hyd"};
+        for (String group: groups) {
+            String result = 
adLdapUserManager.generateADGroupSearchFilter(group, 1L);
+            assertTrue(("(&(objectClass=user)(memberOf=CN=" + group + 
",DC=cloud,DC=citrix,DC=com))").equals(result));
+        }
+    }
+
+    @Mock
+    LdapContext ldapContext;
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testGetUsersInGroupUsingNullGroup() throws Exception {
+        String[] returnAttributes = {"username", "firstname", "lastname", 
"email"};
+        
when(ldapConfiguration.getScope()).thenReturn(SearchControls.SUBTREE_SCOPE);
+        
when(ldapConfiguration.getReturnAttributes(null)).thenReturn(returnAttributes);
+        
when(ldapConfiguration.getBaseDn(any())).thenReturn(null).thenReturn(null).thenReturn("DC=cloud,DC=citrix,DC=com");
+
+        LdapContext context = ldapContext;
+        String [] groups = {null, "group", null};
+        for (String group: groups) {
+            adLdapUserManager.getUsersInGroup(group, context,null);
+        }
+    }
+}
diff --git 
a/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/LdapAuthenticatorTest.java
 
b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/LdapAuthenticatorTest.java
new file mode 100644
index 00000000000..85fd01a0cae
--- /dev/null
+++ 
b/plugins/user-authenticators/ldap/test/org/apache/cloudstack/ldap/LdapAuthenticatorTest.java
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.ldap;
+
+
+import com.cloud.server.auth.UserAuthenticator;
+import com.cloud.user.UserAccount;
+import com.cloud.user.UserAccountVO;
+import com.cloud.user.dao.UserAccountDao;
+import com.cloud.utils.Pair;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
+
+
+@RunWith(MockitoJUnitRunner.class)
+public class LdapAuthenticatorTest {
+
+    @Mock
+    LdapManager ldapManager;
+    @Mock
+    UserAccountDao userAccountDao;
+    @Mock
+    UserAccount user = new UserAccountVO();
+
+    LdapAuthenticator ldapAuthenticator;
+    private String username  = "bbanner";
+    private String principal = "cd=bbanner";
+    private String hardcoded = "password";
+    private Long domainId = 1L;
+
+    @Before
+    public void setUp() throws Exception {
+        ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao);
+    }
+
+    @Test
+    public void authenticateWithoutAccount() throws Exception {
+        LdapUser ldapUser = new 
LdapUser(username,"a@b","b","banner",principal,"",false,null);
+        Pair<Boolean, UserAuthenticator.ActionOnFailedAuthentication> rc;
+        when(ldapManager.getUser(username, domainId)).thenReturn(ldapUser);
+        rc = ldapAuthenticator.authenticate(username, "password", domainId, 
user);
+        assertFalse("authentication succeded when it should have failed", 
rc.first());
+        assertEquals("", 
UserAuthenticator.ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT,rc.second());
+    }
+    @Test
+    public void authenticate() throws Exception {
+        LdapUser ldapUser = new LdapUser(username, "a@b", "b", "banner", 
principal, "", false, null);
+        when(ldapManager.getUser(username, domainId)).thenReturn(ldapUser);
+        when(ldapManager.canAuthenticate(principal, hardcoded, 
domainId)).thenReturn(true);
+        Pair<Boolean, UserAuthenticator.ActionOnFailedAuthentication> rc = 
ldapAuthenticator.authenticate(username, hardcoded, domainId, user);
+        assertTrue("authentication failed when it should have succeeded", 
rc.first());
+        assertNull(rc.second());
+    }
+}
\ No newline at end of file


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to