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:12:32 UTC
git commit: updated refs/heads/4.4 to 82d74da
Repository: cloudstack
Updated Branches:
refs/heads/4.4 73c62837b -> 82d74daf6
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/82d74daf
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/82d74daf
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/82d74daf
Branch: refs/heads/4.4
Commit: 82d74daf65f207fbbf63f058acbabd334eb697b7
Parents: 73c6283
Author: Rajani Karuturi <ra...@gmail.com>
Authored: Mon Oct 27 15:14:38 2014 +0530
Committer: Daan Hoogland <da...@onecht.net>
Committed: Thu Nov 20 12:10:15 2014 +0100
----------------------------------------------------------------------
.../cloudstack/ldap/LdapAuthenticator.java | 9 ++++
.../ldap/LdapAuthenticatorSpec.groovy | 53 ++++++++++++++++++++
2 files changed, 62 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/82d74daf/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 afba272..31404d0 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) {
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/82d74daf/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)