You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2014/11/20 12:13:18 UTC
git commit: updated refs/heads/4.3 to 09a3eef
Repository: cloudstack
Updated Branches:
refs/heads/4.3 c3c3bab41 -> 09a3eefba
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 <da...@onecht.net>
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/09a3eefb
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/09a3eefb
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/09a3eefb
Branch: refs/heads/4.3
Commit: 09a3eefba7b4a59a2ec8d2dc328f6ec19715c3d3
Parents: c3c3bab
Author: Rajani Karuturi <ra...@gmail.com>
Authored: Thu Nov 20 14:56:27 2014 +0530
Committer: Daan Hoogland <da...@onecht.net>
Committed: Thu Nov 20 12:13:02 2014 +0100
----------------------------------------------------------------------
.../cloudstack/ldap/LdapAuthenticator.java | 12 ++++-
.../ldap/LdapAuthenticatorSpec.groovy | 53 ++++++++++++++++++++
2 files changed, 63 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/09a3eefb/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
----------------------------------------------------------------------
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 dac917b..86ea3c4 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;
@@ -50,8 +51,15 @@ 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) {
- final UserAccount user = _userAccountDao.getUserAccount(username,
- domainId);
+ 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) {
s_logger.debug("Unable to find user with " + username + " in domain " + domainId);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/09a3eefb/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
----------------------------------------------------------------------
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 416c133..415d907 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)