Skip to content

Commit 82d74da

Browse files
karuturiDaanHoogland
karuturi
authored and
DaanHoogland
committed
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 <[email protected]>
1 parent 73c6283 commit 82d74da

2 files changed

Lines changed: 62 additions & 0 deletions

File tree

‎plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import javax.inject.Inject;
2222

23+
import org.apache.commons.lang.StringUtils;
2324
import org.apache.log4j.Logger;
2425

2526
import com.cloud.server.auth.DefaultUserAuthenticator;
@@ -48,6 +49,14 @@ public LdapAuthenticator(final LdapManager ldapManager, final UserAccountDao use
4849
@Override
4950
public Pair<Boolean, ActionOnFailedAuthentication> authenticate(final String username, final String password, final Long domainId, final Map<String, Object[]> requestParameters) {
5051

52+
if (StringUtils.isEmpty(username)) {
53+
s_logger.debug("username cannot be empty");
54+
return new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
55+
}
56+
if (StringUtils.isEmpty(password)) {
57+
s_logger.debug("password cannot be empty");
58+
return new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
59+
}
5160
final UserAccount user = _userAccountDao.getUserAccount(username, domainId);
5261

5362
if (user == null) {

‎plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,59 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
3737
result == false
3838
}
3939

40+
def "Test a failed authentication due to empty username"() {
41+
given: "We have an LdapManager, userAccountDao and ldapAuthenticator and the user doesn't exist within cloudstack."
42+
LdapManager ldapManager = Mock(LdapManager)
43+
ldapManager.isLdapEnabled() >> true
44+
ldapManager.canAuthenticate(_, _) >> true
45+
46+
UserAccountDao userAccountDao = Mock(UserAccountDao)
47+
userAccountDao.getUserAccount(_, _) >> new UserAccountVO()
48+
49+
def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao)
50+
51+
when: "A user authenticates"
52+
def result = ldapAuthenticator.authenticate("", "password", 0, null)
53+
then: "their authentication fails"
54+
result.first() == false
55+
56+
when: "A user authenticates"
57+
result = ldapAuthenticator.authenticate(null, "password", 0, null)
58+
then: "their authentication fails"
59+
result.first() == false
60+
61+
when: "A user authenticates"
62+
result = ldapAuthenticator.authenticate(null, null, 0, null)
63+
then: "their authentication fails"
64+
result.first() == false
65+
66+
when: "A user authenticates"
67+
result = ldapAuthenticator.authenticate("", "", 0, null)
68+
then: "their authentication fails"
69+
result.first() == false
70+
}
71+
72+
def "Test failed authentication due to empty password"() {
73+
given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator"
74+
def ldapManager = Mock(LdapManager)
75+
ldapManager.isLdapEnabled() >> true
76+
ldapManager.canAuthenticate(_, _) >> true
77+
78+
UserAccountDao userAccountDao = Mock(UserAccountDao)
79+
userAccountDao.getUserAccount(_, _) >> new UserAccountVO()
80+
def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao)
81+
82+
when: "The user authenticates with empty password"
83+
def result = ldapAuthenticator.authenticate("rmurphy", "", 0, null)
84+
then: "their authentication fails"
85+
result.first() == false
86+
87+
when: "The user authenticates with null password"
88+
result = ldapAuthenticator.authenticate("rmurphy", null, 0, null)
89+
then: "their authentication fails"
90+
result.first() == false
91+
}
92+
4093
def "Test failed authentication due to ldap bind being unsuccessful"() {
4194
given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator"
4295
def ldapManager = Mock(LdapManager)

0 commit comments

Comments
 (0)