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/01/29 21:59:36 UTC
[05/11] git commit: updated refs/heads/4.3 to d6bbfbb
Fix findbug issues within LDAP authenticator
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/9776e1af
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/9776e1af
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/9776e1af
Branch: refs/heads/4.3
Commit: 9776e1af1c92486f5081b1ee8fa95cf0edb86b97
Parents: da344bb
Author: Ian Duffy <ia...@ianduffy.ie>
Authored: Sun Jan 26 00:34:25 2014 +0000
Committer: Daan Hoogland <dh...@schubergphilis.com>
Committed: Wed Jan 29 21:31:12 2014 +0100
----------------------------------------------------------------------
.../cloudstack/api/command/LDAPConfigCmd.java | 2 +-
.../api/command/LdapCreateAccountCmd.java | 244 +++++++++----------
.../api/command/LdapImportUsersCmd.java | 17 +-
.../cloudstack/ldap/LdapContextFactory.java | 2 -
.../ldap/LdapAuthenticatorSpec.groovy | 8 +-
.../cloudstack/ldap/LdapManagerImplSpec.groovy | 16 +-
6 files changed, 137 insertions(+), 152 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9776e1af/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java
index 3faf8b7..a4883c5 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java
@@ -209,7 +209,7 @@ public class LDAPConfigCmd extends BaseCmd {
}
private boolean updateLDAP() {
- LdapConfigurationResponse response = _ldapManager.addConfiguration(hostname, port);
+ _ldapManager.addConfiguration(hostname, port);
/**
* There is no query filter now. It is derived from ldap.user.object and ldap.search.group.principle
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9776e1af/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
index 981e72e..b78b484 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
@@ -35,7 +35,6 @@ 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;
@@ -43,125 +42,126 @@ 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";
-
- @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, 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.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.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_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;
-
- public LdapCreateAccountCmd() {
- super();
- }
-
- 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);
- }
-
- @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 {
- 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");
- }
- }
-
- @Override
- public String getCommandName() {
- return s_name;
- }
-
- CallContext getCurrentContext() {
- return CallContext.current();
- }
-
- @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;
- }
+ public static final Logger s_logger = Logger
+ .getLogger(LdapCreateAccountCmd.class.getName());
+ private static final String s_name = "createaccountresponse";
+
+ @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, 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.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.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_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;
+
+ public LdapCreateAccountCmd() {
+ super();
+ }
+
+ 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);
+ }
+
+ @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!@£$%^&*()_+=";
+
+ 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;
+ }
+
+ CallContext getCurrentContext() {
+ return CallContext.current();
+ }
+
+ @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;
+ }
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9776e1af/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java
index 1855d5d..d82276c 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java
@@ -34,7 +34,6 @@ 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.*;
@@ -171,13 +170,15 @@ public class LdapImportUsersCmd extends BaseListCmd {
}
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");
+ 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);
}
+ return password;
}
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9776e1af/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
index ceeed68..c0d5d50 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
@@ -95,8 +95,6 @@ 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);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9776e1af/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..51f8e84 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
@@ -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 == false
+ result.first() == 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 == false
+ result.first() == 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 == false
+ result.first() == 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 == true
+ result.first() == true
}
def "Test that encode doesn't change the input"() {
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9776e1af/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
index 42988e0..c0ca2e8 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
@@ -297,30 +297,16 @@ 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 return all the commands"
+ then: "it must contain commands"
result.size() > 0
- result == cmdList
}
def "Testing of listConfigurations"() {