Revert "Fix findbug issues within LDAP authenticator"

This reverts commit 92b4f66d73.
This commit is contained in:
Daan Hoogland 2014-01-30 14:55:02 +01:00
parent 16c3f53793
commit 5651081ac0
6 changed files with 133 additions and 118 deletions

View File

@ -209,7 +209,7 @@ public class LDAPConfigCmd extends BaseCmd {
}
private boolean updateLDAP() {
_ldapManager.addConfiguration(hostname, port);
LdapConfigurationResponse response = _ldapManager.addConfiguration(hostname, port);
/**
* There is no query filter now. It is derived from ldap.user.object and ldap.search.group.principle

View File

@ -35,6 +35,7 @@ import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.ldap.LdapManager;
import org.apache.cloudstack.ldap.LdapUser;
import org.apache.log4j.Logger;
import org.bouncycastle.util.encoders.Base64;
import com.cloud.user.Account;
import com.cloud.user.AccountService;
@ -42,126 +43,125 @@ import com.cloud.user.UserAccount;
@APICommand(name = "ldapCreateAccount", description = "Creates an account from an LDAP user", responseObject = AccountResponse.class, since = "4.2.0")
public class LdapCreateAccountCmd extends BaseCmd {
public static final Logger s_logger = Logger
.getLogger(LdapCreateAccountCmd.class.getName());
private static final String s_name = "createaccountresponse";
public static final Logger s_logger = Logger
.getLogger(LdapCreateAccountCmd.class.getName());
private static final String s_name = "createaccountresponse";
@Inject
private LdapManager _ldapManager;
@Inject
private LdapManager _ldapManager;
@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "Creates the user under the specified account. If no account is specified, the username will be used as the account name.")
private String accountName;
@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "Creates the user under the specified account. If no account is specified, the username will be used as the account name.")
private String accountName;
@Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.SHORT, required = true, description = "Type of the account. Specify 0 for user, 1 for root admin, and 2 for domain admin")
private Short accountType;
@Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.SHORT, required = true, description = "Type of the account. Specify 0 for user, 1 for root admin, and 2 for domain admin")
private Short accountType;
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Creates the user under the specified domain.")
private Long domainId;
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Creates the user under the specified domain.")
private Long domainId;
@Parameter(name = ApiConstants.TIMEZONE, type = CommandType.STRING, description = "Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.")
private String timezone;
@Parameter(name = ApiConstants.TIMEZONE, type = CommandType.STRING, description = "Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.")
private String timezone;
@Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, required = true, description = "Unique username.")
private String username;
@Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, required = true, description = "Unique username.")
private String username;
@Parameter(name = ApiConstants.NETWORK_DOMAIN, type = CommandType.STRING, description = "Network domain for the account's networks")
private String networkDomain;
@Parameter(name = ApiConstants.NETWORK_DOMAIN, type = CommandType.STRING, description = "Network domain for the account's networks")
private String networkDomain;
@Parameter(name = ApiConstants.ACCOUNT_DETAILS, type = CommandType.MAP, description = "details for account used to store specific parameters")
private Map<String, String> details;
@Parameter(name = ApiConstants.ACCOUNT_DETAILS, type = CommandType.MAP, description = "details for account used to store specific parameters")
private Map<String, String> details;
@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.STRING, description = "Account UUID, required for adding account from external provisioning system")
private String accountUUID;
@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.STRING, description = "Account UUID, required for adding account from external provisioning system")
private String accountUUID;
@Parameter(name = ApiConstants.USER_ID, type = CommandType.STRING, description = "User UUID, required for adding account from external provisioning system")
private String userUUID;
@Parameter(name = ApiConstants.USER_ID, type = CommandType.STRING, description = "User UUID, required for adding account from external provisioning system")
private String userUUID;
public LdapCreateAccountCmd() {
super();
}
public LdapCreateAccountCmd() {
super();
}
public LdapCreateAccountCmd(final LdapManager ldapManager,
final AccountService accountService) {
super();
_ldapManager = ldapManager;
_accountService = accountService;
}
public LdapCreateAccountCmd(final LdapManager ldapManager,
final AccountService accountService) {
super();
_ldapManager = ldapManager;
_accountService = accountService;
}
UserAccount createCloudstackUserAccount(final LdapUser user) {
return _accountService.createUserAccount(username, generatePassword(),
user.getFirstname(), user.getLastname(), user.getEmail(),
timezone, accountName, accountType, domainId, networkDomain,
details, accountUUID, userUUID);
}
UserAccount createCloudstackUserAccount(final LdapUser user) {
return _accountService.createUserAccount(username, generatePassword(),
user.getFirstname(), user.getLastname(), user.getEmail(),
timezone, accountName, accountType, domainId, networkDomain,
details, accountUUID, userUUID);
}
@Override
public void execute() throws ServerApiException {
final CallContext callContext = getCurrentContext();
callContext.setEventDetails("Account Name: " + accountName
+ ", Domain Id:" + domainId);
try {
final LdapUser user = _ldapManager.getUser(username);
validateUser(user);
final UserAccount userAccount = createCloudstackUserAccount(user);
if (userAccount != null) {
final AccountResponse response = _responseGenerator
.createUserAccountResponse(userAccount);
response.setResponseName(getCommandName());
setResponseObject(response);
} else {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
"Failed to create a user account");
}
} catch (final NamingException e) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR,
"No LDAP user exists with the username of " + username);
}
}
@Override
public void execute() throws ServerApiException {
final CallContext callContext = getCurrentContext();
callContext.setEventDetails("Account Name: " + accountName
+ ", Domain Id:" + domainId);
try {
final LdapUser user = _ldapManager.getUser(username);
validateUser(user);
final UserAccount userAccount = createCloudstackUserAccount(user);
if (userAccount != null) {
final AccountResponse response = _responseGenerator
.createUserAccountResponse(userAccount);
response.setResponseName(getCommandName());
setResponseObject(response);
} else {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
"Failed to create a user account");
}
} catch (final NamingException e) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR,
"No LDAP user exists with the username of " + username);
}
}
private String generatePassword() throws ServerApiException {
final SecureRandom random = new SecureRandom();
final int length = 20;
final String characters = "abcdefghjkmnpqrstuvwxyzABCDEFGHJKMNPQRSTUVWXYZ23456789!@£$%^&*()_+=";
private String generatePassword() throws ServerApiException {
try {
final SecureRandom randomGen = SecureRandom.getInstance("SHA1PRNG");
final byte bytes[] = new byte[20];
randomGen.nextBytes(bytes);
return Base64.encode(bytes).toString();
} catch (final NoSuchAlgorithmException e) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
"Failed to generate random password");
}
}
String password = "";
for (int i = 0; i < length; i++) {
int index = (int) (random.nextDouble() * characters.length());
password += characters.charAt(index);
}
return password;
}
@Override
public String getCommandName() {
return s_name;
}
@Override
public String getCommandName() {
return s_name;
}
CallContext getCurrentContext() {
return CallContext.current();
}
CallContext getCurrentContext() {
return CallContext.current();
}
@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
}
@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
}
private boolean validateUser(final LdapUser user) throws ServerApiException {
if (user.getEmail() == null) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username
+ " has no email address set within LDAP");
}
if (user.getFirstname() == null) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username
+ " has no firstname set within LDAP");
}
if (user.getLastname() == null) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username
+ " has no lastname set within LDAP");
}
return true;
}
private boolean validateUser(final LdapUser user) throws ServerApiException {
if (user.getEmail() == null) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username
+ " has no email address set within LDAP");
}
if (user.getFirstname() == null) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username
+ " has no firstname set within LDAP");
}
if (user.getLastname() == null) {
throw new ServerApiException(
ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username
+ " has no lastname set within LDAP");
}
return true;
}
}

View File

@ -34,6 +34,7 @@ import org.apache.cloudstack.ldap.LdapUser;
import org.apache.cloudstack.ldap.NoLdapUserMatchingQueryException;
import org.apache.commons.lang.StringUtils;
import org.apache.log4j.Logger;
import org.bouncycastle.util.encoders.Base64;
import com.cloud.domain.Domain;
import com.cloud.exception.*;
@ -170,15 +171,13 @@ public class LdapImportUsersCmd extends BaseListCmd {
}
private String generatePassword() throws ServerApiException {
final SecureRandom random = new SecureRandom();
final int length = 20;
final String characters = "abcdefghjkmnpqrstuvwxyzABCDEFGHJKMNPQRSTUVWXYZ23456789!@£$%^&*()_+=";
String password = "";
for (int i = 0; i < length; i++) {
int index = (int) (random.nextDouble() * characters.length());
password += characters.charAt(index);
try {
final SecureRandom randomGen = SecureRandom.getInstance("SHA1PRNG");
final byte bytes[] = new byte[20];
randomGen.nextBytes(bytes);
return Base64.encode(bytes).toString();
} catch (final NoSuchAlgorithmException e) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to generate random password");
}
return password;
}
}

View File

@ -95,6 +95,8 @@ public class LdapContextFactory {
environment.put(Context.INITIAL_CONTEXT_FACTORY, factory);
environment.put(Context.PROVIDER_URL, url);
environment.put("com.sun.jndi.ldap.read.timeout", "500");
environment.put("com.sun.jndi.ldap.connect.pool", "true");
enableSSL(environment);
setAuthentication(environment, isSystemContext);

View File

@ -34,7 +34,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
when: "A user authentications"
def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
then: "their authentication fails"
result.first() == false
result == false
}
def "Test failed authentication due to ldap bind being unsuccessful"() {
@ -51,7 +51,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
then: "their authentication fails"
result.first() == false
result == false
}
def "Test failed authentication due to ldap not being configured"() {
@ -66,7 +66,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
when: "The user authenticates"
def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
then: "their authentication fails"
result.first() == false
result == false
}
def "Test successful authentication"() {
@ -83,7 +83,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
then: "their authentication passes"
result.first() == true
result == true
}
def "Test that encode doesn't change the input"() {

View File

@ -297,16 +297,30 @@ class LdapManagerImplSpec extends spock.lang.Specification {
thrown InvalidParameterValueException
}
def supportedLdapCommands() {
List<Class<?>> cmdList = new ArrayList<Class<?>>();
cmdList.add(LdapUserSearchCmd.class);
cmdList.add(LdapListUsersCmd.class);
cmdList.add(LdapAddConfigurationCmd.class);
cmdList.add(LdapDeleteConfigurationCmd.class);
cmdList.add(LdapListConfigurationCmd.class);
cmdList.add(LdapCreateAccountCmd.class);
cmdList.add(LdapImportUsersCmd.class);
return cmdList
}
def "Test that getCommands isn't empty"() {
given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
def ldapContextFactory = Mock(LdapContextFactory)
def ldapUserManager = Mock(LdapUserManager)
final List<Class<?>> cmdList = supportedLdapCommands()
def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
when: "Get commands is called"
def result = ldapManager.getCommands()
then: "it must contain commands"
then: "it must return all the commands"
result.size() > 0
result == cmdList
}
def "Testing of listConfigurations"() {