From 82d74daf65f207fbbf63f058acbabd334eb697b7 Mon Sep 17 00:00:00 2001
From: Rajani Karuturi <rajanikaruturi@gmail.com>
Date: Mon, 27 Oct 2014 15:14:38 +0530
Subject: [PATCH] Fixed CLOUDSTACK-7937 CloudStack accepts unauthenticated LDAP
 binds

added validation checks for empty username and password and debug logs
when that happens.

Signed-off-by: Daan Hoogland <daan@onecht.net>
---
 .../cloudstack/ldap/LdapAuthenticator.java    |  9 ++++
 .../ldap/LdapAuthenticatorSpec.groovy         | 53 +++++++++++++++++++
 2 files changed, 62 insertions(+)

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 afba27238a4..31404d01087 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
@@ -20,6 +20,7 @@ import java.util.Map;
 
 import javax.inject.Inject;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.server.auth.DefaultUserAuthenticator;
@@ -48,6 +49,14 @@ public class LdapAuthenticator extends DefaultUserAuthenticator {
     @Override
     public Pair<Boolean, ActionOnFailedAuthentication> authenticate(final String username, final String password, final Long domainId, final Map<String, Object[]> requestParameters) {
 
+        if (StringUtils.isEmpty(username)) {
+            s_logger.debug("username cannot be empty");
+            return new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
+        }
+        if (StringUtils.isEmpty(password)) {
+            s_logger.debug("password cannot be empty");
+            return new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
+        }
         final UserAccount user = _userAccountDao.getUserAccount(username, domainId);
 
         if (user == null) {
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
index 416c1330359..415d9077983 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
@@ -37,6 +37,59 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
 		result == false
     }
 
+    def "Test a failed authentication due to empty username"() {
+        given: "We have an LdapManager, userAccountDao and ldapAuthenticator and the user doesn't exist within cloudstack."
+        LdapManager ldapManager = Mock(LdapManager)
+        ldapManager.isLdapEnabled() >> true
+        ldapManager.canAuthenticate(_, _) >> true
+
+        UserAccountDao userAccountDao = Mock(UserAccountDao)
+        userAccountDao.getUserAccount(_, _) >> new UserAccountVO()
+
+        def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao)
+
+        when: "A user authenticates"
+        def result = ldapAuthenticator.authenticate("", "password", 0, null)
+        then: "their authentication fails"
+        result.first() == false
+
+        when: "A user authenticates"
+        result = ldapAuthenticator.authenticate(null, "password", 0, null)
+        then: "their authentication fails"
+        result.first() == false
+
+        when: "A user authenticates"
+        result = ldapAuthenticator.authenticate(null, null, 0, null)
+        then: "their authentication fails"
+        result.first() == false
+
+        when: "A user authenticates"
+        result = ldapAuthenticator.authenticate("", "", 0, null)
+        then: "their authentication fails"
+        result.first() == false
+    }
+
+    def "Test failed authentication due to empty password"() {
+        given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator"
+        def ldapManager = Mock(LdapManager)
+        ldapManager.isLdapEnabled() >> true
+        ldapManager.canAuthenticate(_, _) >> true
+
+        UserAccountDao userAccountDao = Mock(UserAccountDao)
+        userAccountDao.getUserAccount(_, _) >> new UserAccountVO()
+        def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao)
+
+        when: "The user authenticates with empty password"
+        def result = ldapAuthenticator.authenticate("rmurphy", "", 0, null)
+        then: "their authentication fails"
+        result.first() == false
+
+        when: "The user authenticates with null password"
+        result = ldapAuthenticator.authenticate("rmurphy", null, 0, null)
+        then: "their authentication fails"
+        result.first() == false
+    }
+
     def "Test failed authentication due to ldap bind being unsuccessful"() {
 		given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator"
 		def ldapManager = Mock(LdapManager)