Updated Branches: refs/heads/4.3-forward 16c3f5379 -> 5651081ac
Revert "Fix findbug issues within LDAP authenticator" This reverts commit 92b4f66d73562e4211d2d787554ff229dbeb5705. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/5651081a Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/5651081a Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/5651081a Branch: refs/heads/4.3-forward Commit: 5651081ac0a32ba88c7446f079a5d3a8dca47feb Parents: 16c3f53 Author: Daan Hoogland <dhoogl...@schubergphilis.com> Authored: Thu Jan 30 14:55:02 2014 +0100 Committer: Daan Hoogland <dhoogl...@schubergphilis.com> Committed: Thu Jan 30 14:55:02 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, 152 insertions(+), 137 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5651081a/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 a4883c5..3faf8b7 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() { - _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 http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5651081a/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 b78b484..981e72e 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,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"; - - @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; - } + 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; + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5651081a/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 d82276c..1855d5d 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,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; } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5651081a/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 c0d5d50..ceeed68 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,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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5651081a/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 51f8e84..416c133 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.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"() { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5651081a/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 c0ca2e8..42988e0 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,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"() {
