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>
This commit is contained in:
Rajani Karuturi 2014-11-20 14:56:27 +05:30 committed by Daan Hoogland
parent c3c3bab41a
commit 09a3eefba7
2 changed files with 63 additions and 2 deletions

View File

@ -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);

View File

@ -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)