diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapAddConfigurationCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapAddConfigurationCmd.java index 1131667d98ae..59d68a3f07f2 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapAddConfigurationCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapAddConfigurationCmd.java @@ -46,7 +46,7 @@ public class LdapAddConfigurationCmd extends BaseCmd { @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, required = true, description = "Port") private int port; - @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = false, entityType = DomainResponse.class, description = "linked domain") + @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "linked domain") private Long domainId; public LdapAddConfigurationCmd() { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java index 880ecea4d13c..2715cc196e19 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java @@ -21,6 +21,7 @@ import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.user.UserAccount; +import com.cloud.utils.StringUtils; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -39,7 +40,6 @@ import org.bouncycastle.util.encoders.Base64; import javax.inject.Inject; -import java.io.UnsupportedEncodingException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Map; @@ -107,7 +107,7 @@ public Account.Type getAccountType() { if (accountType == null) { return RoleType.getAccountTypeByRole(roleService.findRole(roleId), null); } - return RoleType.getAccountTypeByRole(roleService.findRole(roleId), Account.Type.getFromValue(accountType.intValue())); + return RoleType.getAccountTypeByRole(roleService.findRole(roleId), Account.Type.getFromValue(accountType)); } public Long getRoleId() { @@ -158,10 +158,10 @@ public void execute() throws ServerApiException { private String generatePassword() throws ServerApiException { try { final SecureRandom randomGen = SecureRandom.getInstance("SHA1PRNG"); - final byte bytes[] = new byte[20]; + final byte[] bytes = new byte[20]; randomGen.nextBytes(bytes); - return new String(Base64.encode(bytes), "UTF-8"); - } catch (NoSuchAlgorithmException | UnsupportedEncodingException e) { + return new String(Base64.encode(bytes), StringUtils.getPreferredCharset()); + } catch (NoSuchAlgorithmException e) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to generate random password"); } } @@ -180,7 +180,7 @@ public long getEntityOwnerId() { return Account.ACCOUNT_ID_SYSTEM; } - private boolean validateUser(final LdapUser user) throws ServerApiException { + private void 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"); } @@ -190,6 +190,5 @@ private boolean validateUser(final LdapUser user) throws ServerApiException { if (user.getLastname() == null) { throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, username + " has no lastname set within LDAP"); } - return true; } } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapDeleteConfigurationCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapDeleteConfigurationCmd.java index 0ce7daff432b..ffbe224df410 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapDeleteConfigurationCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapDeleteConfigurationCmd.java @@ -46,10 +46,10 @@ public class LdapDeleteConfigurationCmd extends BaseCmd { @Parameter(name = ApiConstants.HOST_NAME, type = CommandType.STRING, description = "Hostname") private String hostname; - @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, required = false, description = "port") + @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "port") private int port; - @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = false, entityType = DomainResponse.class, description = "linked domain") + @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "linked domain") private Long domainId; public LdapDeleteConfigurationCmd() { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapImportUsersCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapImportUsersCmd.java index eada5f6df39b..1a14040a31bf 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapImportUsersCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapImportUsersCmd.java @@ -16,7 +16,6 @@ // under the License. package org.apache.cloudstack.api.command; -import java.io.UnsupportedEncodingException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.ArrayList; @@ -41,7 +40,6 @@ import org.apache.cloudstack.ldap.LdapManager; import org.apache.cloudstack.ldap.LdapUser; import org.apache.cloudstack.ldap.NoLdapUserMatchingQueryException; -import org.apache.commons.lang3.StringUtils; import org.bouncycastle.util.encoders.Base64; import com.cloud.domain.Domain; @@ -56,6 +54,7 @@ import com.cloud.user.DomainService; import com.cloud.user.User; import com.cloud.user.UserAccount; +import com.cloud.utils.StringUtils; @APICommand(name = "importLdapUsers", description = "Import LDAP users", responseObject = LdapUserResponse.class, since = "4.3.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class LdapImportUsersCmd extends BaseListCmd { @@ -106,14 +105,14 @@ public LdapImportUsersCmd(final LdapManager ldapManager, final DomainService dom private void createCloudstackUserAccount(LdapUser user, String accountName, Domain domain) { Account account = _accountService.getActiveAccountByName(accountName, domain.getId()); if (account == null) { - logger.debug("No account exists with name: " + accountName + " creating the account and an user with name: " + user.getUsername() + " in the account"); + logger.debug("No account exists with name: {}. Creating the account and a user with name: {} in the account", accountName, user.getUsername()); _accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, getAccountType(), getRoleId(), domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); } else { // check if the user exists. if yes, call update UserAccount csuser = _accountService.getActiveUserAccount(user.getUsername(), domain.getId()); if (csuser == null) { - logger.debug("No user exists with name: " + user.getUsername() + " creating a user in the account: " + accountName); + logger.debug("No user exists with name: {}. Creating a user in the account: {}", user.getUsername(), accountName); _accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(), UUID.randomUUID().toString(), User.Source.LDAP); } else { @@ -145,21 +144,21 @@ public void execute() users = _ldapManager.getUsers(domainId); } } catch (NoLdapUserMatchingQueryException ex) { - users = new ArrayList(); - logger.info("No Ldap user matching query. " + " ::: " + ex.getMessage()); + users = new ArrayList<>(); + logger.info("No Ldap user matching query. ::: {}", ex.getMessage()); } - List addedUsers = new ArrayList(); + List addedUsers = new ArrayList<>(); for (LdapUser user : users) { Domain domain = getDomain(user); try { createCloudstackUserAccount(user, getAccountName(user), domain); addedUsers.add(user); } catch (InvalidParameterValueException ex) { - logger.error("Failed to create user with username: " + user.getUsername() + " ::: " + ex.getMessage()); + logger.error("Failed to create user with username: {} ::: {}", user.getUsername(), ex.getMessage()); } } - ListResponse response = new ListResponse(); + ListResponse response = new ListResponse<>(); response.setResponses(createLdapUserResponse(addedUsers)); response.setResponseName(getCommandName()); setResponseObject(response); @@ -169,7 +168,7 @@ public Account.Type getAccountType() { if (accountType == null) { return RoleType.getAccountTypeByRole(roleService.findRole(roleId), null); } - return RoleType.getAccountTypeByRole(roleService.findRole(roleId), Account.Type.getFromValue(accountType.intValue())); + return RoleType.getAccountTypeByRole(roleService.findRole(roleId), Account.Type.getFromValue(accountType)); } public Long getRoleId() { @@ -202,11 +201,11 @@ private Domain getDomainForName(String name) { private Domain getDomain(LdapUser user) { Domain domain; if (_domain != null) { - //this means either domain id or groupname is passed and this will be same for all the users in this call. hence returning it. + // This means that either a domain id or group name is passed and this will be same for all the users in this call, hence returning it. domain = _domain; } else { if (domainId != null) { - // a domain Id is passed. use it for this user and all the users in the same api call (by setting _domain) + // a domain ID is passed. Use it for this user and all the users in the same API call (by setting _domain) domain = _domain = _domainService.getDomain(domainId); } else { // a group name is passed. use it for this user and all the users in the same api call(by setting _domain) @@ -225,7 +224,7 @@ private Domain getDomain(LdapUser user) { } private List createLdapUserResponse(List users) { - final List ldapResponses = new ArrayList(); + final List ldapResponses = new ArrayList<>(); for (final LdapUser user : users) { final LdapUserResponse ldapResponse = _ldapManager.createLdapUserResponse(user); ldapResponse.setObjectName("LdapUser"); @@ -242,10 +241,10 @@ public String getCommandName() { private String generatePassword() throws ServerApiException { try { final SecureRandom randomGen = SecureRandom.getInstance("SHA1PRNG"); - final byte bytes[] = new byte[20]; + final byte[] bytes = new byte[20]; randomGen.nextBytes(bytes); - return new String(Base64.encode(bytes), "UTF-8"); - } catch (NoSuchAlgorithmException | UnsupportedEncodingException e) { + return new String(Base64.encode(bytes), StringUtils.getPreferredCharset()); + } catch (NoSuchAlgorithmException e) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to generate random password"); } } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListConfigurationCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListConfigurationCmd.java index c950554b0d1f..25653a9c0a3f 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListConfigurationCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListConfigurationCmd.java @@ -44,20 +44,22 @@ public class LdapListConfigurationCmd extends BaseListCmd { @Inject private LdapManager _ldapManager; - @Parameter(name = ApiConstants. HOST_NAME, type = CommandType.STRING, required = false, description = "Hostname") + @Parameter(name = ApiConstants. HOST_NAME, type = CommandType.STRING, description = "Hostname") private String hostname; - @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, required = false, description = "Port") + @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port") private int port; - @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = false, entityType = DomainResponse.class, description = "linked domain") + @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, + description = "linked domain") private Long domainId; @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = LdapConfigurationResponse.class, description = "list ldap configuration by ID; when passed, all other parameters are ignored") private Long id; - @Parameter(name = ApiConstants.LIST_ALL, type = CommandType.BOOLEAN, description = "If set to true, " - + " and no domainid specified, list all LDAP configurations irrespective of the linked domain", since = "4.13.2") + @Parameter(name = ApiConstants.LIST_ALL, type = CommandType.BOOLEAN, + description = "If set to true, and no `domainid` specified, list all LDAP configurations irrespective of the linked domain", + since = "4.13.2") private Boolean listAll; public LdapListConfigurationCmd() { @@ -70,7 +72,7 @@ public LdapListConfigurationCmd(final LdapManager ldapManager) { } private List createLdapConfigurationResponses(final List configurations) { - final List responses = new ArrayList(); + final List responses = new ArrayList<>(); for (final LdapConfigurationVO resource : configurations) { final LdapConfigurationResponse configurationResponse = _ldapManager.createLdapConfigurationResponse(resource); configurationResponse.setObjectName("LdapConfiguration"); @@ -83,7 +85,7 @@ private List createLdapConfigurationResponses(final L public void execute() { final Pair, Integer> result = _ldapManager.listConfigurations(this); final List responses = createLdapConfigurationResponses(result.first()); - final ListResponse response = new ListResponse(); + final ListResponse response = new ListResponse<>(); response.setResponses(responses, result.second()); response.setResponseName(getCommandName()); setResponseObject(response); diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java index 6bfc70ea139c..f5f6e00614ef 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java @@ -16,10 +16,9 @@ // under the License. package org.apache.cloudstack.api.command; -import java.lang.reflect.ParameterizedType; -import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import javax.inject.Inject; @@ -44,9 +43,10 @@ import org.apache.cloudstack.query.QueryService; import com.cloud.user.Account; +import org.apache.commons.collections.CollectionUtils; /** - * a short flow, use plantuml to view (see http://plantuml.com) + * a short flow, use plantuml to view (see the plantuml site) * @startuml * start * :list ldap users request; @@ -84,14 +84,12 @@ public class LdapListUsersCmd extends BaseListCmd { @Parameter(name = "listtype", type = CommandType.STRING, - required = false, description = "Determines whether all ldap users are returned or just non-cloudstack users. This option is deprecated in favour for the more option rich 'userfilter' parameter") @Deprecated private String listType; @Parameter(name = ApiConstants.USER_FILTER, type = CommandType.STRING, - required = false, since = "4.13", description = "Determines what type of filter is applied on the list of users returned from LDAP.\n" + "\tvalid values are\n" @@ -102,7 +100,7 @@ public class LdapListUsersCmd extends BaseListCmd { + " including those that are already in cloudstack, the later will be annotated with their userSource") private String userFilter; - @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = false, entityType = DomainResponse.class, description = "linked domain") + @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "linked domain") private Long domainId; public LdapListUsersCmd() { @@ -121,7 +119,7 @@ public LdapListUsersCmd(final LdapManager ldapManager, final QueryService queryS * @return a (filtered?) list of user response objects */ private List createLdapUserResponse(final List users) { - final List ldapResponses = new ArrayList(); + final List ldapResponses = new ArrayList<>(); for (final LdapUser user : users) { final LdapUserResponse ldapResponse = _ldapManager.createLdapUserResponse(user); ldapResponse.setObjectName("LdapUser"); @@ -135,8 +133,8 @@ private List createLdapUserResponse(final List users @Override public void execute() throws ServerApiException { cloudstackUsers = null; - List ldapResponses = new ArrayList(); - final ListResponse response = new ListResponse(); + List ldapResponses = new ArrayList<>(); + final ListResponse response = new ListResponse<>(); try { final List users = _ldapManager.getUsers(domainId); ldapResponses = createLdapUserResponse(users); @@ -177,16 +175,13 @@ private void traceUserList() { users.append(user.getUsername()); } - logger.trace(String.format("checking against %d cloudstackusers: %s.", this.cloudstackUsers.size(), users.toString())); + logger.trace("checking against {} cloudstackusers: {}.", this.cloudstackUsers.size(), users); } } private List applyUserFilter(List ldapResponses) { - if(logger.isTraceEnabled()) { - logger.trace(String.format("applying filter: %s or %s.", this.getListTypeString(), this.getUserFilter())); - } - List responseList = getUserFilter().filter(this,ldapResponses); - return responseList; + logger.trace("applying filter: {} or {}.", this.getListTypeString(), this.getUserFilter()); + return getUserFilter().filter(this,ldapResponses); } @Override @@ -211,48 +206,34 @@ UserFilter getUserFilter() { return UserFilter.fromString(getUserFilterString()); } - boolean isACloudstackUser(final LdapUser ldapUser) { + boolean isACloudStackUser(final LdapUser ldapUser) { + String username = ldapUser.getUsername(); + return isACloudStackUser(username); + } + + boolean isACloudStackUser(final LdapUserResponse ldapUser) { + logger.trace("checking response : {}", ldapUser.toString()); + String username = ldapUser.getUsername(); + return isACloudStackUser(username); + } + + private boolean isACloudStackUser(String username) { boolean rc = false; final List cloudstackUsers = getCloudstackUsers(); - if (cloudstackUsers != null) { + if (CollectionUtils.isNotEmpty(cloudstackUsers)) { for (final UserResponse cloudstackUser : cloudstackUsers) { - if (ldapUser.getUsername().equals(cloudstackUser.getUsername())) { - if(logger.isTraceEnabled()) { - logger.trace(String.format("found user %s in cloudstack", ldapUser.getUsername())); - } - + if (username.equals(cloudstackUser.getUsername())) { + logger.trace("Found user {} in CloudStack", cloudstackUser.getUsername()); rc = true; + break; } else { - if(logger.isTraceEnabled()) { - logger.trace(String.format("ldap user %s does not match cloudstack user %s", ldapUser.getUsername(), cloudstackUser.getUsername())); - } + logger.trace("ldap user {} does not match cloudstack user {}", username, cloudstackUser.getUsername()); } } } return rc; } - boolean isACloudstackUser(final LdapUserResponse ldapUser) { - if(logger.isTraceEnabled()) { - logger.trace("checking response : " + ldapUser.toString()); - } - final List cloudstackUsers = getCloudstackUsers(); - if (cloudstackUsers != null && cloudstackUsers.size() != 0) { - for (final UserResponse cloudstackUser : cloudstackUsers) { - if (ldapUser.getUsername().equals(cloudstackUser.getUsername())) { - if(logger.isTraceEnabled()) { - logger.trace(String.format("found user %s in cloudstack user %s", ldapUser.getUsername(), cloudstackUser.getUsername())); - } - return true; - } else { - if(logger.isTraceEnabled()) { - logger.trace(String.format("ldap user %s does not match cloudstack user %s", ldapUser.getUsername(), cloudstackUser.getUsername())); - } - } - } - } - return false; - } /** * typecheck for userfilter values and filter type dependend functionalities. * This could have been in two switch statements elsewhere in the code. @@ -363,7 +344,7 @@ public List filterAnyDomain(List input) { if(logger.isTraceEnabled()) { logger.trace("filtering existing users"); } - final List ldapResponses = new ArrayList(); + final List ldapResponses = new ArrayList<>(); for (final LdapUserResponse user : input) { if (isNotAlreadyImportedInTheCurrentDomain(user)) { ldapResponses.add(user); @@ -396,7 +377,7 @@ public List filterLocalDomain(List input) { if(logger.isTraceEnabled()) { logger.trace("filtering local domain users"); } - final List ldapResponses = new ArrayList(); + final List ldapResponses = new ArrayList<>(); String domainId = getCurrentDomainId(); for (final LdapUserResponse user : input) { UserResponse cloudstackUser = getCloudstackUser(user); @@ -412,7 +393,7 @@ public List filterLocalDomain(List input) { } private String getCurrentDomainId() { - String domainId = null; + String domainId; if (this.domainId != null) { Domain domain = _domainService.getDomain(this.domainId); domainId = domain.getUuid(); @@ -433,10 +414,10 @@ public List filterPotentialImport(List input logger.trace("should be filtering potential imports!!!"); } // functional possibility do not add only users not yet in cloudstack but include users that would be moved if they are so in ldap? - // this means if they are part of a account linked to an ldap group/ou + // This means if they are part of an Account linked to an LDAP Group/OU input.removeIf(ldapUser -> ( - (isACloudstackUser(ldapUser)) + (isACloudStackUser(ldapUser)) && (getCloudstackUser(ldapUser).getUserSource().equalsIgnoreCase(User.Source.LDAP.toString())) ) ); @@ -466,7 +447,7 @@ private UserResponse getCloudstackUser(LdapUserResponse user) { for (final UserResponse cloudstackUser : cloudstackUsers) { if (user.getUsername().equals(cloudstackUser.getUsername())) { returnObject = cloudstackUser; - if (returnObject.getDomainId() == this.getCurrentDomainId()) { + if (Objects.equals(returnObject.getDomainId(), this.getCurrentDomainId())) { break; } } @@ -474,31 +455,4 @@ private UserResponse getCloudstackUser(LdapUserResponse user) { } return returnObject; } - - private void checkFilterMethodType(Type returnType) { - String msg = null; - if (returnType instanceof ParameterizedType) { - ParameterizedType type = (ParameterizedType) returnType; - if(type.getRawType().equals(List.class)) { - Type[] typeArguments = type.getActualTypeArguments(); - if (typeArguments.length == 1) { - if (typeArguments[0].equals(LdapUserResponse.class)) { - // we're good' - } else { - msg = new String("list of return type contains " + typeArguments[0].getTypeName()); - } - } else { - msg = String.format("type %s has to the wrong number of arguments", type.getRawType()); - } - } else { - msg = String.format("type %s is not a List<>", type.getTypeName()); - } - } else { - msg = new String("can't even begin to explain; review your method signature"); - } - if(msg != null) { - throw new IllegalArgumentException(msg); - } - } - } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapUserSearchCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapUserSearchCmd.java index b702beda1708..d26c3c259bfa 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapUserSearchCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapUserSearchCmd.java @@ -54,7 +54,7 @@ public LdapUserSearchCmd(final LdapManager ldapManager) { } private List createLdapUserResponse(final List users) { - final List ldapUserResponses = new ArrayList(); + final List ldapUserResponses = new ArrayList<>(); if (users != null) { for (final LdapUser user : users) { final LdapUserResponse ldapUserResponse = _ldapManager.createLdapUserResponse(user); @@ -67,7 +67,7 @@ private List createLdapUserResponse(final List users @Override public void execute() { - final ListResponse response = new ListResponse(); + final ListResponse response = new ListResponse<>(); List users = null; try { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmd.java index 52ece5c44f43..ffc4b473b7bc 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmd.java @@ -50,7 +50,7 @@ public class LinkAccountToLdapCmd extends BaseCmd { @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = true, entityType = DomainResponse.class, description = "The id of the domain that is to contain the linked account.") private Long domainId; - @Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, required = false, description = "type of the ldap name. GROUP or OU, defaults to GROUP") + @Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, description = "type of the ldap name. GROUP or OU, defaults to GROUP") private String type; @Parameter(name = ApiConstants.LDAP_DOMAIN, type = CommandType.STRING, required = true, description = "name of the group or OU in LDAP") @@ -59,14 +59,14 @@ public class LinkAccountToLdapCmd extends BaseCmd { @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, required = true, description = "name of the account, it will be created if it does not exist") private String accountName; - @Parameter(name = ApiConstants.ADMIN, type = CommandType.STRING, required = false, description = "domain admin username in LDAP ") + @Parameter(name = ApiConstants.ADMIN, type = CommandType.STRING, description = "domain admin username in LDAP ") private String admin; - @Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.INTEGER, required = false, description = "Type of the account to auto import. Specify 0 for user and 2 for " + @Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.INTEGER, description = "Type of the account to auto import. Specify 0 for user and 2 for " + "domain admin") private Integer accountType; - @Parameter(name = ApiConstants.ROLE_ID, type = CommandType.UUID, entityType = RoleResponse.class, required = false, description = "Creates the account under the specified role.", since="4.19.1") + @Parameter(name = ApiConstants.ROLE_ID, type = CommandType.UUID, entityType = RoleResponse.class, description = "Creates the account under the specified role.", since="4.19.1") private Long roleId; @Inject @@ -81,7 +81,7 @@ public void execute() throws ServerApiException { try { ldapUser = _ldapManager.getUser(admin, type, ldapDomain, domainId); } catch (NoLdapUserMatchingQueryException e) { - logger.debug("no ldap user matching username " + admin + " in the given group/ou", e); + logger.debug("no ldap user matching username {} in the given group/ou", admin, e); } if (ldapUser != null && !ldapUser.isDisabled()) { Account account = _accountService.getActiveAccountByName(admin, domainId); @@ -99,7 +99,7 @@ public void execute() throws ServerApiException { logger.debug("an account with name {} already exists in the domain {} with id {}", admin, _domainService.getDomain(domainId), domainId); } } else { - logger.debug("ldap user with username " + admin + " is disabled in the given group/ou"); + logger.debug("ldap user with username {} is disabled in the given group/ou", admin); } } response.setObjectName(this.getActualCommandName()); @@ -139,7 +139,7 @@ public Account.Type getAccountType() { if (accountType == null) { return RoleType.getAccountTypeByRole(roleService.findRole(roleId), null); } - return RoleType.getAccountTypeByRole(roleService.findRole(roleId), Account.Type.getFromValue(accountType.intValue())); + return RoleType.getAccountTypeByRole(roleService.findRole(roleId), Account.Type.getFromValue(accountType)); } public Long getRoleId() { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java index b6c32fe49b5e..12209526d885 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -111,7 +111,7 @@ public void execute() throws ServerApiException { logger.debug("an account with name {} already exists in the domain {} with id {}", admin, _domainService.getDomain(domainId), domainId); } } else { - logger.debug("ldap user with username {} is disabled in the given group/ou", admin); + logger.debug("ldap user with username {} is disabled in the given Group/OU", admin); } } response.setObjectName("LinkDomainToLdap"); diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/response/LinkAccountToLdapResponse.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/response/LinkAccountToLdapResponse.java index 6273273bdbff..6366ee9176e1 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/response/LinkAccountToLdapResponse.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/response/LinkAccountToLdapResponse.java @@ -82,4 +82,8 @@ public String getAdminId() { public void setAdminId(String adminId) { this.adminId = adminId; } + + public String getAccountName() { + return accountName; + } } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java index bf5d503e8416..3f8aa6a4c514 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java @@ -39,7 +39,7 @@ public List getUsersInGroup(String groupName, LdapContext context, Lon throw new IllegalArgumentException("ldap group name cannot be blank"); } - String basedn = _ldapConfiguration.getBaseDn(domainId); + String basedn = LdapConfiguration.getBaseDn(domainId); if (StringUtils.isBlank(basedn)) { throw new IllegalArgumentException("ldap basedn is not configured"); } @@ -62,7 +62,7 @@ String generateADGroupSearchFilter(String groupName, Long domainId) { final StringBuilder userObjectFilter = getUserObjectFilter(domainId); final StringBuilder memberOfFilter = new StringBuilder(); - String groupCnName = _ldapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + _ldapConfiguration.getBaseDn(domainId); + String groupCnName = LdapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + LdapConfiguration.getBaseDn(domainId); memberOfFilter.append("(").append(getMemberOfAttribute(domainId)).append("="); memberOfFilter.append(groupCnName); memberOfFilter.append(")"); @@ -100,7 +100,7 @@ protected boolean isUserDisabled(SearchResult result) throws NamingException { protected String getMemberOfAttribute(final Long domainId) { String rc; - if(_ldapConfiguration.isNestedGroupsEnabled(domainId)) { + if(LdapConfiguration.isNestedGroupsEnabled(domainId)) { rc = MICROSOFT_AD_NESTED_MEMBERS_FILTER; } else { rc = MICROSOFT_AD_MEMBERS_FILTER; diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java index 36c663566cb9..09519c5641cb 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.auth.UserAuthenticator; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; import com.cloud.user.Account; @@ -59,10 +60,10 @@ public LdapAuthenticator(final LdapManager ldapManager, final UserAccountDao use @Override public Pair authenticate(final String username, final String password, final Long domainId, final Map requestParameters) { - Pair rc = new Pair(false, null); + Pair rc = new Pair<>(false, null); if (logger.isDebugEnabled()) { - logger.debug("Retrieving ldap user: " + username); + logger.debug("Retrieving ldap user: {}", username); } // TODO not allowing an empty password is a policy we shouldn't decide on. A private cloud may well want to allow this. @@ -76,7 +77,7 @@ public Pair authenticate(final String use return rc; } List ldapTrustMapVOs = getLdapTrustMapVOS(domainId); - if (ldapTrustMapVOs != null && ldapTrustMapVOs.size() > 0) { + if (CollectionUtils.isNotEmpty(ldapTrustMapVOs)) { if (ldapTrustMapVOs.size() == 1 && ldapTrustMapVOs.get(0).getAccountId() == 0) { if (logger.isTraceEnabled()) { logger.trace("We have a single mapping of a domain to an ldap group or ou"); @@ -90,7 +91,7 @@ public Pair authenticate(final String use } } else { if (logger.isTraceEnabled()) { - logger.trace(String.format("'this' domain (%d) is not linked to ldap follow normal authentication", domainId)); + logger.trace("'this' domain ({}) is not linked to LDAP follow normal authentication", domainId); } rc = authenticate(username, password, domainId, user); } @@ -114,10 +115,10 @@ private List getLdapTrustMapVOS(Long domainId) { * @param domainId domain the user is trying to log on to * @param userAccount cloudstack user object * @param ldapTrustMapVOs the trust mappings of accounts in the domain to ldap groups - * @return false if the ldap user object does not exist, is not mapped to an account, is mapped to multiple accounts or if authenitication fails + * @return {false,} if the ldap user object does not exist, is not mapped to an account, is mapped to multiple accounts or if authentication fails */ Pair authenticate(String username, String password, Long domainId, UserAccount userAccount, List ldapTrustMapVOs) { - Pair rc = new Pair(false, null); + Pair rc = new Pair<>(false, null); try { LdapUser ldapUser = _ldapManager.getUser(username, domainId); List memberships = ldapUser.getMemberships(); @@ -131,7 +132,7 @@ Pair authenticate(String username, String logAndDisable(userAccount, "attempt to log on using disabled ldap user " + userAccount.getUsername(), false); } else if (mappedGroups.size() > 1) { logAndDisable(userAccount, "user '" + username + "' is mapped to more then one account in domain and will be disabled.", false); - } else if (mappedGroups.size() < 1) { + } else if (mappedGroups.isEmpty()) { logAndDisable(userAccount, "user '" + username + "' is not mapped to an account in domain and will be removed.", true); } else { // a valid ldap configured user exists @@ -230,10 +231,10 @@ List getMappedGroups(List ldapTrustMapVOs) { * @param domainId domain the user is trying to log on to * @param user cloudstack user object * @param ldapTrustMapVO the trust mapping for the domain to the ldap group - * @return false if the ldap user object does not exist or authenitication fails + * @return {false, } if the ldap user object does not exist or authentication fails */ private Pair authenticate(String username, String password, Long domainId, UserAccount user, LdapTrustMapVO ldapTrustMapVO) { - Pair rc = new Pair(false, null); + Pair rc = new Pair<>(false, null); try { LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType().toString(), ldapTrustMapVO.getName(), domainId); final Account.Type accountType = ldapTrustMapVO.getAccountType(); @@ -270,7 +271,7 @@ private void processLdapUser(String password, Long domainId, UserAccount user, P * @param password pass phrase * @param domainId domain the user is trying to log on to * @param user cloudstack user object - * @return false if either user object does not exist or authenitication fails + * @return {false,} if either user object does not exist or authentication fails */ Pair authenticate(String username, String password, Long domainId, UserAccount user) { boolean result = false; @@ -282,7 +283,7 @@ Pair authenticate(String username, String if (!ldapUser.isDisabled()) { result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password, domainId); } else { - logger.debug("user with principal "+ ldapUser.getPrincipal() + " is disabled in ldap"); + logger.debug("user with principal {} is disabled in ldap", () -> ldapUser.getPrincipal()); } } catch (NoLdapUserMatchingQueryException e) { logger.debug(e.getMessage()); @@ -296,8 +297,8 @@ Pair authenticate(String username, String private Pair processResultAndAction(UserAccount user, boolean result, boolean timedOut) { return (!result && (user != null || timedOut)) ? - new Pair(result, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT): - new Pair(result, null); + new Pair<>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT): + new Pair<>(result, null); } private void enableUserInCloudStack(UserAccount user) { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapConfiguration.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapConfiguration.java index 87ff2d0a2acd..114ca1631ec5 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapConfiguration.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapConfiguration.java @@ -23,7 +23,6 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import com.cloud.utils.Pair; import org.apache.cloudstack.ldap.dao.LdapConfigurationDao; @@ -34,7 +33,7 @@ public class LdapConfiguration implements Configurable{ private final static String factory = "com.sun.jndi.ldap.LdapCtxFactory"; protected Logger logger = LogManager.getLogger(getClass()); - private static final ConfigKey ldapReadTimeout = new ConfigKey( + private static final ConfigKey ldapReadTimeout = new ConfigKey<>( Long.class, "ldap.read.timeout", "Advanced", @@ -42,9 +41,9 @@ public class LdapConfiguration implements Configurable{ "LDAP connection Timeout in milli sec", true, ConfigKey.Scope.Domain, - 1l); + 1L); - private static final ConfigKey ldapPageSize = new ConfigKey( + private static final ConfigKey ldapPageSize = new ConfigKey<>( Integer.class, "ldap.request.page.size", "Advanced", @@ -54,7 +53,7 @@ public class LdapConfiguration implements Configurable{ ConfigKey.Scope.Domain, 1); - private static final ConfigKey ldapEnableNestedGroups = new ConfigKey( + private static final ConfigKey ldapEnableNestedGroups = new ConfigKey<>( "Advanced", Boolean.class, "ldap.nested.groups.enable", @@ -63,7 +62,7 @@ public class LdapConfiguration implements Configurable{ true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapMemberOfAttribute = new ConfigKey( + private static final ConfigKey ldapMemberOfAttribute = new ConfigKey<>( "Advanced", String.class, "ldap.user.memberof.attribute", @@ -72,7 +71,7 @@ public class LdapConfiguration implements Configurable{ true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapProvider = new ConfigKey( + private static final ConfigKey ldapProvider = new ConfigKey<>( "Advanced", String.class, "ldap.provider", @@ -81,7 +80,7 @@ public class LdapConfiguration implements Configurable{ true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapBaseDn = new ConfigKey( + private static final ConfigKey ldapBaseDn = new ConfigKey<>( "Advanced", String.class, "ldap.basedn", @@ -90,7 +89,7 @@ public class LdapConfiguration implements Configurable{ true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapBindPassword = new ConfigKey( + private static final ConfigKey ldapBindPassword = new ConfigKey<>( "Secure", String.class, "ldap.bind.password", @@ -98,7 +97,7 @@ public class LdapConfiguration implements Configurable{ "Sets the bind password for LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapBindPrincipal = new ConfigKey( + private static final ConfigKey ldapBindPrincipal = new ConfigKey<>( "Secure", String.class, "ldap.bind.principal", @@ -106,7 +105,7 @@ public class LdapConfiguration implements Configurable{ "Sets the bind principal for LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapEmailAttribute = new ConfigKey( + private static final ConfigKey ldapEmailAttribute = new ConfigKey<>( "Advanced", String.class, "ldap.email.attribute", @@ -114,7 +113,7 @@ public class LdapConfiguration implements Configurable{ "Sets the email attribute used within LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapFirstnameAttribute = new ConfigKey( + private static final ConfigKey ldapFirstnameAttribute = new ConfigKey<>( "Advanced", String.class, "ldap.firstname.attribute", @@ -122,14 +121,14 @@ public class LdapConfiguration implements Configurable{ "Sets the firstname attribute used within LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapLastnameAttribute = new ConfigKey( + private static final ConfigKey ldapLastnameAttribute = new ConfigKey<>( "Advanced", String.class, "ldap.lastname.attribute", "sn", "Sets the lastname attribute used within LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapUsernameAttribute = new ConfigKey( + private static final ConfigKey ldapUsernameAttribute = new ConfigKey<>( "Advanced", String.class, "ldap.username.attribute", @@ -137,7 +136,7 @@ public class LdapConfiguration implements Configurable{ "Sets the username attribute used within LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapUserObject = new ConfigKey( + private static final ConfigKey ldapUserObject = new ConfigKey<>( "Advanced", String.class, "ldap.user.object", @@ -145,7 +144,7 @@ public class LdapConfiguration implements Configurable{ "Sets the object type of users within LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapSearchGroupPrinciple = new ConfigKey( + private static final ConfigKey ldapSearchGroupPrinciple = new ConfigKey<>( "Advanced", String.class, "ldap.search.group.principle", @@ -153,7 +152,7 @@ public class LdapConfiguration implements Configurable{ "Sets the principle of the group that users must be a member of", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapGroupObject = new ConfigKey( + private static final ConfigKey ldapGroupObject = new ConfigKey<>( "Advanced", String.class, "ldap.group.object", @@ -161,7 +160,7 @@ public class LdapConfiguration implements Configurable{ "Sets the object type of groups within LDAP", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapGroupUniqueMemberAttribute = new ConfigKey( + private static final ConfigKey ldapGroupUniqueMemberAttribute = new ConfigKey<>( "Advanced", String.class, "ldap.group.user.uniquemember", @@ -170,7 +169,7 @@ public class LdapConfiguration implements Configurable{ true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapTrustStore = new ConfigKey( + private static final ConfigKey ldapTrustStore = new ConfigKey<>( "Advanced", String.class, "ldap.truststore", @@ -178,7 +177,7 @@ public class LdapConfiguration implements Configurable{ "Sets the path to the truststore to use for SSL", true, ConfigKey.Scope.Domain); - private static final ConfigKey ldapTrustStorePassword = new ConfigKey( + private static final ConfigKey ldapTrustStorePassword = new ConfigKey<>( "Secure", String.class, "ldap.truststore.password", @@ -199,11 +198,6 @@ public LdapConfiguration(final LdapConfigurationDao ldapConfigurationDao) { _ldapConfigurationDao = ldapConfigurationDao; } - @Deprecated - public LdapConfiguration(final ConfigurationDao configDao, final LdapConfigurationDao ldapConfigurationDao) { - _ldapConfigurationDao = ldapConfigurationDao; - } - public String getAuthentication(final Long domainId) { if ((getBindPrincipal(domainId) == null) && (getBindPassword(domainId) == null)) { return "none"; @@ -212,36 +206,36 @@ public String getAuthentication(final Long domainId) { } } - public String getBaseDn(final Long domainId) { + public static String getBaseDn(final Long domainId) { return ldapBaseDn.valueIn(domainId); } - public String getBindPassword(final Long domainId) { + public static String getBindPassword(final Long domainId) { return ldapBindPassword.valueIn(domainId); } - public String getBindPrincipal(final Long domainId) { + public static String getBindPrincipal(final Long domainId) { return ldapBindPrincipal.valueIn(domainId); } - public String getEmailAttribute(final Long domainId) { + public static String getEmailAttribute(final Long domainId) { return ldapEmailAttribute.valueIn(domainId); } - public String getFactory() { + public static String getFactory() { return factory; } - public String getFirstnameAttribute(final Long domainId) { + public static String getFirstnameAttribute(final Long domainId) { return ldapFirstnameAttribute.valueIn(domainId); } - public String getLastnameAttribute(final Long domainId) { + public static String getLastnameAttribute(final Long domainId) { return ldapLastnameAttribute.valueIn(domainId); } public String getProviderUrl(final Long domainId) { - final String protocol = getSSLStatus(domainId) == true ? "ldaps://" : "ldap://"; + final String protocol = getSSLStatus(domainId) ? "ldaps://" : "ldap://"; final Pair, Integer> result = _ldapConfigurationDao.searchConfigurations(null, 0, domainId); final StringBuilder providerUrls = new StringBuilder(); String delim = ""; @@ -273,40 +267,36 @@ public String getSearchGroupPrinciple(final Long domainId) { return ldapSearchGroupPrinciple.valueIn(domainId); } - public boolean getSSLStatus(Long domainId) { - boolean sslStatus = false; - if (getTrustStore(domainId) != null && getTrustStorePassword(domainId) != null) { - sslStatus = true; - } - return sslStatus; + public static boolean getSSLStatus(Long domainId) { + return getTrustStore(domainId) != null && getTrustStorePassword(domainId) != null; } - public String getTrustStore(Long domainId) { + public static String getTrustStore(Long domainId) { return ldapTrustStore.valueIn(domainId); } - public String getTrustStorePassword(Long domainId) { + public static String getTrustStorePassword(Long domainId) { return ldapTrustStorePassword.valueIn(domainId); } - public String getUsernameAttribute(final Long domainId) { + public static String getUsernameAttribute(final Long domainId) { return ldapUsernameAttribute.valueIn(domainId); } - public String getUserObject(final Long domainId) { + public static String getUserObject(final Long domainId) { return ldapUserObject.valueIn(domainId); } - public String getGroupObject(final Long domainId) { + public static String getGroupObject(final Long domainId) { return ldapGroupObject.valueIn(domainId); } - public String getGroupUniqueMemberAttribute(final Long domainId) { + public static String getGroupUniqueMemberAttribute(final Long domainId) { return ldapGroupUniqueMemberAttribute.valueIn(domainId); } // TODO remove hard-coding - public String getCommonNameAttribute() { + public static String getCommonNameAttribute() { return "cn"; } @@ -315,11 +305,11 @@ public String getUserAccountControlAttribute() { return "userAccountControl"; } - public Long getReadTimeout(final Long domainId) { + public static Long getReadTimeout(final Long domainId) { return ldapReadTimeout.valueIn(domainId); } - public Integer getLdapPageSize(final Long domainId) { + public static Integer getLdapPageSize(final Long domainId) { return ldapPageSize.valueIn(domainId); } @@ -334,7 +324,7 @@ public LdapUserManager.Provider getLdapProvider(final Long domainId) { return provider; } - public boolean isNestedGroupsEnabled(final Long domainId) { + public static boolean isNestedGroupsEnabled(final Long domainId) { return ldapEnableNestedGroups.valueIn(domainId); } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapContextFactory.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapContextFactory.java index 1c759c6a45ed..34ebc7a44991 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapContextFactory.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapContextFactory.java @@ -49,8 +49,8 @@ public LdapContext createBindContext(Long domainId) throws NamingException, IOEx } public LdapContext createBindContext(final String providerUrl, Long domainId) throws NamingException, IOException { - final String bindPrincipal = _ldapConfiguration.getBindPrincipal(domainId); - final String bindPassword = _ldapConfiguration.getBindPassword(domainId); + final String bindPrincipal = LdapConfiguration.getBindPrincipal(domainId); + final String bindPassword = LdapConfiguration.getBindPassword(domainId); return createInitialDirContext(bindPrincipal, bindPassword, providerUrl, true, domainId); } @@ -70,13 +70,13 @@ public LdapContext createUserContext(final String principal, final String passwo } private void enableSSL(final Hashtable environment, Long domainId) { - final boolean sslStatus = _ldapConfiguration.getSSLStatus(domainId); + final boolean sslStatus = LdapConfiguration.getSSLStatus(domainId); if (sslStatus) { logger.info("LDAP SSL enabled."); environment.put(Context.SECURITY_PROTOCOL, "ssl"); - String trustStore = _ldapConfiguration.getTrustStore(domainId); - String trustStorePassword = _ldapConfiguration.getTrustStorePassword(domainId); + String trustStore = LdapConfiguration.getTrustStore(domainId); + String trustStorePassword = LdapConfiguration.getTrustStorePassword(domainId); if (!validateTrustStore(trustStore, trustStorePassword)) { throw new RuntimeException("Invalid truststore or truststore password"); @@ -109,7 +109,7 @@ private boolean validateTrustStore(String trustStore, String trustStorePassword) } private Hashtable getEnvironment(final String principal, final String password, final String providerUrl, final boolean isSystemContext, Long domainId) { - final String factory = _ldapConfiguration.getFactory(); + final String factory = LdapConfiguration.getFactory(); String url = providerUrl == null ? _ldapConfiguration.getProviderUrl(domainId) : providerUrl; if (StringUtils.isEmpty(url) && domainId != null) { //try a default ldap implementation @@ -120,7 +120,7 @@ private Hashtable getEnvironment(final String principal, final S environment.put(Context.INITIAL_CONTEXT_FACTORY, factory); environment.put(Context.PROVIDER_URL, url); - environment.put("com.sun.jndi.ldap.read.timeout", _ldapConfiguration.getReadTimeout(domainId).toString()); + environment.put("com.sun.jndi.ldap.read.timeout", LdapConfiguration.getReadTimeout(domainId).toString()); environment.put("com.sun.jndi.ldap.connect.pool", "true"); enableSSL(environment, domainId); diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java index ac50a8130c13..7e561ccf754a 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java @@ -35,7 +35,7 @@ public interface LdapManager extends PluggableService { - enum LinkType { GROUP, OU} + enum LinkType { GROUP, OU } LdapConfigurationResponse addConfiguration(final LdapAddConfigurationCmd cmd) throws InvalidParameterValueException; @@ -76,8 +76,6 @@ enum LinkType { GROUP, OU} List getDomainLinkage(long domainId); - LdapTrustMapVO getAccountLinkedToLdap(long domainId, long accountId); - LdapTrustMapVO getLinkedLdapGroup(long domainId, String group); LinkAccountToLdapResponse linkAccountToLdap(LinkAccountToLdapCmd linkAccountToLdapCmd); diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java index a139688700a4..8327072639c7 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java @@ -49,11 +49,11 @@ import org.apache.cloudstack.api.response.LinkAccountToLdapResponse; import org.apache.cloudstack.api.response.LinkDomainToLdapResponse; import org.apache.cloudstack.framework.messagebus.MessageBus; -import org.apache.cloudstack.framework.messagebus.MessageSubscriber; import org.apache.cloudstack.ldap.dao.LdapConfigurationDao; import org.apache.cloudstack.ldap.dao.LdapTrustMapDao; import org.apache.commons.lang.Validate; import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; import org.springframework.stereotype.Component; import com.cloud.domain.DomainVO; @@ -115,36 +115,30 @@ public boolean configure(String name, Map params) throws Configu } private void addAccountRemovalListener() { - messageBus.subscribe(AccountManager.MESSAGE_REMOVE_ACCOUNT_EVENT, new MessageSubscriber() { - @Override - public void onPublishMessage(String senderAddress, String subject, Object args) { - try { - final Account account = accountDao.findByIdIncludingRemoved((Long) args); - long domainId = account.getDomainId(); - LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.findByAccount(domainId, account.getAccountId()); - if (ldapTrustMapVO != null) { - removeTrustmap(ldapTrustMapVO); - } - } catch (final Exception e) { - logger.error("Caught exception while removing account linked to LDAP", e); + messageBus.subscribe(AccountManager.MESSAGE_REMOVE_ACCOUNT_EVENT, (senderAddress, subject, args) -> { + try { + final Account account = accountDao.findByIdIncludingRemoved((Long) args); + long domainId = account.getDomainId(); + LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.findByAccount(domainId, account.getAccountId()); + if (ldapTrustMapVO != null) { + removeTrustmap(ldapTrustMapVO); } + } catch (final Exception e) { + logger.error("Caught exception while removing account linked to LDAP", e); } }); } private void addDomainRemovalListener() { - messageBus.subscribe(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT, new MessageSubscriber() { - @Override - public void onPublishMessage(String senderAddress, String subject, Object args) { - try { - long domainId = ((DomainVO) args).getId(); - List ldapTrustMapVOs = _ldapTrustMapDao.searchByDomainId(domainId); - for (LdapTrustMapVO ldapTrustMapVO : ldapTrustMapVOs) { - removeTrustmap(ldapTrustMapVO); - } - } catch (final Exception e) { - logger.error("Caught exception while removing trust-map for domain linked to LDAP", e); + messageBus.subscribe(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT, (senderAddress, subject, args) -> { + try { + long domainId = ((DomainVO) args).getId(); + List ldapTrustMapVOs = _ldapTrustMapDao.searchByDomainId(domainId); + for (LdapTrustMapVO ldapTrustMapVO : ldapTrustMapVOs) { + removeTrustmap(ldapTrustMapVO); } + } catch (final Exception e) { + logger.error("Caught exception while removing trust-map for domain linked to LDAP", e); } }); } @@ -181,7 +175,7 @@ private LdapConfigurationResponse addConfigurationInternal(final String hostname context = _ldapContextFactory.createBindContext(providerUrl,domainId); configuration = new LdapConfigurationVO(hostname, port, domainId); _ldapConfigurationDao.persist(configuration); - logger.info("Added new ldap server with url: " + providerUrl + (domainId == null ? "": " for domain " + domainId)); + logger.info("Added a new LDAP server with URL: {}{}", providerUrl, domainId == null ? "" : " for domain " + domainId); return createLdapConfigurationResponse(configuration); } catch (NamingException | IOException e) { logger.debug("NamingException while doing an LDAP bind", e); @@ -201,10 +195,10 @@ private LdapConfigurationResponse addConfigurationInternal(final String hostname /** * TODO decide if the principal is good enough to get the domain id or we need to add it as parameter - * @param principal - * @param password - * @param domainId - * @return + * @param principal ldap user + * @param password the users password to check + * @param domainId the domain for logging into + * @return true if the user can authenticate */ @Override public boolean canAuthenticate(final String principal, final String password, final Long domainId) { @@ -212,13 +206,11 @@ public boolean canAuthenticate(final String principal, final String password, fi // TODO return the right account for this user final LdapContext context = _ldapContextFactory.createUserContext(principal, password, domainId); closeContext(context); - if (logger.isTraceEnabled()) { - logger.trace(String.format("User(%s) authenticated for domain(%s)", principal, domainId)); - } + logger.trace("User({}) authenticated for domain({})", principal, domainId); return true; } catch (NamingException | IOException e) {/* AuthenticationException is caught as NamingException */ - logger.debug("Exception while doing an LDAP bind for user "+" "+principal, e); - logger.info("Failed to authenticate user: " + principal + ". incorrect password."); + logger.debug("Exception while doing an LDAP bind for user {}", principal, e); + logger.info("Failed to authenticate user: {}. Incorrect password.", principal); return false; } } @@ -286,7 +278,7 @@ private LdapConfigurationResponse deleteConfigurationInternal(final String hostn throw new InvalidParameterValueException("Cannot find configuration with hostname " + hostname); } else { _ldapConfigurationDao.remove(configuration.getId()); - logger.info("Removed ldap server with url: " + hostname + ':' + port + (domainId == null ? "" : " for domain id " + domainId)); + logger.info("Removed ldap server with url: {}:{}{}", hostname, port, domainId == null ? "" : " for domain id " + domainId); return createLdapConfigurationResponse(configuration); } } @@ -417,7 +409,7 @@ public List searchUsers(final String username) throws NoLdapUserMatchi @Override public LinkDomainToLdapResponse linkDomainToLdap(LinkDomainToLdapCmd cmd) { final Long domainId = cmd.getDomainId(); - final String baseDn = _ldapConfiguration.getBaseDn(domainId); + final String baseDn = LdapConfiguration.getBaseDn(domainId); final String ldapDomain = cmd.getLdapDomain(); Validate.isTrue(baseDn != null, String.format("can not link a domain (with id = %d) unless a basedn (%s) is configured for it.", domainId, baseDn)); @@ -437,16 +429,42 @@ private LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, St //Account type should be 0 or 2. check the constants in com.cloud.user.Account Validate.isTrue(accountType== Account.Type.NORMAL || accountType== Account.Type.DOMAIN_ADMIN, "accountype should be either 0(normal user) or 2(domain admin)"); LinkType linkType = LdapManager.LinkType.valueOf(type.toUpperCase()); - LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, linkType, name, accountType, 0)); - DomainVO domain = domainDao.findById(vo.getDomainId()); - String domainUuid = ""; + return linkDomainToLdapAndGetResponse(domainId, name, accountType, linkType); + } + + @NotNull + private LinkDomainToLdapResponse linkDomainToLdapAndGetResponse(Long domainId, String name, Account.Type accountType, LinkType linkType) { + DomainVO domain = getDomainToLink(domainId); + LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(domain.getId(), linkType, name, accountType, 0)); + String domainUuid = domain.getUuid(); + return new LinkDomainToLdapResponse(domainUuid, vo.getType().toString(), vo.getName(), vo.getAccountType().ordinal()); + } + + @NotNull + private DomainVO getDomainToLink(Long domainId) { + DomainVO domain = domainDao.findById(domainId); if (domain == null) { - logger.error("no domain in database for id " + vo.getDomainId()); - } else { - domainUuid = domain.getUuid(); + String msg = "Cannot link Domain to LDAP. No domain found"; + logger.error(msg); + throw new InvalidParameterValueException(msg); } - LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainUuid, vo.getType().toString(), vo.getName(), vo.getAccountType().ordinal()); - return response; + return domain; + } + + @NotNull + private LinkAccountToLdapResponse linkAccountToLdapAndGetResponse(LinkAccountToLdapCmd cmd) { + DomainVO domain = getDomainToLink(cmd.getDomainId()); + LinkType linkType = LinkType.valueOf(cmd.getType().toUpperCase()); + Account account = accountDao.findActiveAccount(cmd.getAccountName(), cmd.getDomainId()); + if (account == null) { + account = new AccountVO(cmd.getAccountName(), cmd.getDomainId(), null, cmd.getAccountType(), cmd.getRoleId(), UUID.randomUUID().toString()); + accountDao.persist((AccountVO)account); + } + + long accountId = account.getAccountId(); + clearOldAccountMapping(cmd); + LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(cmd.getDomainId(), linkType, cmd.getLdapDomain(), cmd.getAccountType(), accountId)); + return new LinkAccountToLdapResponse(domain.getUuid(), vo.getType().toString(), vo.getName(), vo.getAccountType().ordinal(), account.getUuid(), cmd.getAccountName()); } private boolean unlinkDomainFromLdap(Long domainId) { @@ -468,10 +486,6 @@ public List getDomainLinkage(long domainId){ return _ldapTrustMapDao.searchByDomainId(domainId); } - public LdapTrustMapVO getAccountLinkedToLdap(long domainId, long accountId){ - return _ldapTrustMapDao.findByAccount(domainId, accountId); - } - @Override public LdapTrustMapVO getLinkedLdapGroup(long domainId, String group) { return _ldapTrustMapDao.findGroupInDomain(domainId, group); @@ -479,7 +493,7 @@ public LdapTrustMapVO getLinkedLdapGroup(long domainId, String group) { @Override public LinkAccountToLdapResponse linkAccountToLdap(LinkAccountToLdapCmd cmd) { - Validate.notNull(_ldapConfiguration.getBaseDn(cmd.getDomainId()), "can not link an account to ldap in a domain for which no basdn is configured"); + Validate.notNull(LdapConfiguration.getBaseDn(cmd.getDomainId()), "can not link an account to ldap in a domain for which no basdn is configured"); Validate.notNull(cmd.getDomainId(), "domainId cannot be null."); Validate.notEmpty(cmd.getAccountName(), "accountName cannot be empty."); Validate.notEmpty(cmd.getLdapDomain(), "ldapDomain cannot be empty, please supply a GROUP or OU name"); @@ -487,26 +501,7 @@ public LinkAccountToLdapResponse linkAccountToLdap(LinkAccountToLdapCmd cmd) { Validate.notEmpty(cmd.getLdapDomain(), "GROUP or OU name cannot be empty"); Validate.isTrue(cmd.getAccountType() != null || cmd.getRoleId() != null, "Either account type or role ID must be given"); - LinkType linkType = LdapManager.LinkType.valueOf(cmd.getType().toUpperCase()); - Account account = accountDao.findActiveAccount(cmd.getAccountName(),cmd.getDomainId()); - if (account == null) { - account = new AccountVO(cmd.getAccountName(), cmd.getDomainId(), null, cmd.getAccountType(), cmd.getRoleId(), UUID.randomUUID().toString()); - accountDao.persist((AccountVO)account); - } - - Long accountId = account.getAccountId(); - clearOldAccountMapping(cmd); - LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(cmd.getDomainId(), linkType, cmd.getLdapDomain(), cmd.getAccountType(), accountId)); - DomainVO domain = domainDao.findById(vo.getDomainId()); - String domainUuid = ""; - if (domain == null) { - logger.error("no domain in database for id " + vo.getDomainId()); - } else { - domainUuid = domain.getUuid(); - } - - LinkAccountToLdapResponse response = new LinkAccountToLdapResponse(domainUuid, vo.getType().toString(), vo.getName(), vo.getAccountType().ordinal(), account.getUuid(), cmd.getAccountName()); - return response; + return linkAccountToLdapAndGetResponse(cmd); } private void clearOldAccountMapping(LinkAccountToLdapCmd cmd) { @@ -514,7 +509,7 @@ private void clearOldAccountMapping(LinkAccountToLdapCmd cmd) { LdapTrustMapVO oldVo = _ldapTrustMapDao.findGroupInDomain(cmd.getDomainId(), cmd.getLdapDomain()); if (oldVo != null) { // deal with edge cases, i.e. check if the old account is indeed deleted etc. - if (oldVo.getAccountId() != 0l) { + if (oldVo.getAccountId() != 0L) { AccountVO oldAcount = accountDao.findByIdIncludingRemoved(oldVo.getAccountId()); String msg = String.format("group %s is mapped to account %d in the current domain (%s)", cmd.getLdapDomain(), oldVo.getAccountId(), cmd.getDomainId()); if (null == oldAcount.getRemoved()) { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapTrustMapVO.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapTrustMapVO.java index 27125998e4c5..c04c16a71ca6 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapTrustMapVO.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapTrustMapVO.java @@ -123,8 +123,8 @@ public boolean equals(Object o) { public int hashCode() { int result = type.hashCode(); result = 31 * result + name.hashCode(); - result = 31 * result + (int) (domainId ^ (domainId >>> 32)); - result = 31 * result + (int) (accountId ^ (accountId >>> 32)); + result = 31 * result + Long.hashCode(domainId); + result = 31 * result + Long.hashCode(accountId); result = 31 * result + accountType.ordinal(); return result; } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUser.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUser.java index d2b58656dc58..20b13ff70a8b 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUser.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUser.java @@ -27,7 +27,7 @@ public class LdapUser implements Comparable { private final String username; private final String domain; private final boolean disabled; - private List memberships; + private final List memberships; public LdapUser(final String username, final String email, final String firstname, final String lastname, final String principal, String domain, boolean disabled, List memberships) { diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUserManager.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUserManager.java index c9fcaa23cc04..e9c7db093da9 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUserManager.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapUserManager.java @@ -26,21 +26,21 @@ public interface LdapUserManager { - public enum Provider { - MICROSOFTAD, OPENLDAP; + enum Provider { + MICROSOFTAD, OPENLDAP } - public LdapUser getUser(final String username, final LdapContext context, Long domainId) throws NamingException, IOException; + LdapUser getUser(final String username, final LdapContext context, Long domainId) throws NamingException, IOException; - public LdapUser getUser(final String username, final String type, final String name, final LdapContext context, Long domainId) throws NamingException, IOException; + LdapUser getUser(final String username, final String type, final String name, final LdapContext context, Long domainId) throws NamingException, IOException; - public List getUsers(final LdapContext context, Long domainId) throws NamingException, IOException; + List getUsers(final LdapContext context, Long domainId) throws NamingException, IOException; - public List getUsers(final String username, final LdapContext context, Long domainId) throws NamingException, IOException; + List getUsers(final String username, final LdapContext context, Long domainId) throws NamingException, IOException; - public List getUsersInGroup(String groupName, LdapContext context, Long domainId) throws NamingException; + List getUsersInGroup(String groupName, LdapContext context, Long domainId) throws NamingException; - public List searchUsers(final LdapContext context, Long domainId) throws NamingException, IOException; + List searchUsers(final LdapContext context, Long domainId) throws NamingException, IOException; - public List searchUsers(final String username, final LdapContext context, Long domainId) throws NamingException, IOException; + List searchUsers(final String username, final LdapContext context, Long domainId) throws NamingException, IOException; } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java index 80d394d7478c..50ba2f34ae3b 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java @@ -58,15 +58,15 @@ public OpenLdapUserManagerImpl(final LdapConfiguration ldapConfiguration) { protected LdapUser createUser(final SearchResult result, Long domainId) throws NamingException { final Attributes attributes = result.getAttributes(); - final String username = LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getUsernameAttribute(domainId)); - final String email = LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getEmailAttribute(domainId)); - final String firstname = LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getFirstnameAttribute(domainId)); - final String lastname = LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getLastnameAttribute(domainId)); + final String username = LdapUtils.getAttributeValue(attributes, LdapConfiguration.getUsernameAttribute(domainId)); + final String email = LdapUtils.getAttributeValue(attributes, LdapConfiguration.getEmailAttribute(domainId)); + final String firstname = LdapUtils.getAttributeValue(attributes, LdapConfiguration.getFirstnameAttribute(domainId)); + final String lastname = LdapUtils.getAttributeValue(attributes, LdapConfiguration.getLastnameAttribute(domainId)); final String principal = result.getNameInNamespace(); final List memberships = LdapUtils.getAttributeValues(attributes, getMemberOfAttribute(domainId)); - String domain = principal.replace("cn=" + LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getCommonNameAttribute()) + ",", ""); - domain = domain.replace("," + _ldapConfiguration.getBaseDn(domainId), ""); + String domain = principal.replace("cn=" + LdapUtils.getAttributeValue(attributes, LdapConfiguration.getCommonNameAttribute()) + ",", ""); + domain = domain.replace("," + LdapConfiguration.getBaseDn(domainId), ""); domain = domain.replace("ou=", ""); boolean disabled = isUserDisabled(result); @@ -83,7 +83,7 @@ private String generateSearchFilter(final String username, Long domainId) { StringBuilder ldapGroupsFilter = new StringBuilder(); // this should get the trustmaps for this domain List ldapGroups = getMappedLdapGroups(domainId); - if (!ldapGroups.isEmpty()) { + if (CollectionUtils.isNotEmpty(ldapGroups)) { ldapGroupsFilter.append("(|"); for (String ldapGroup : ldapGroups) { ldapGroupsFilter.append(getMemberOfGroupString(ldapGroup, memberOfAttribute)); @@ -110,7 +110,7 @@ private String generateSearchFilter(final String username, Long domainId) { private StringBuilder getUsernameFilter(String username, Long domainId) { final StringBuilder usernameFilter = new StringBuilder(); usernameFilter.append("("); - usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId)); + usernameFilter.append(LdapConfiguration.getUsernameAttribute(domainId)); usernameFilter.append("="); usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username))); usernameFilter.append(")"); @@ -120,7 +120,7 @@ private StringBuilder getUsernameFilter(String username, Long domainId) { StringBuilder getUserObjectFilter(Long domainId) { final StringBuilder userObjectFilter = new StringBuilder(); userObjectFilter.append("(objectClass="); - userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); + userObjectFilter.append(LdapConfiguration.getUserObject(domainId)); userObjectFilter.append(")"); return userObjectFilter; } @@ -152,11 +152,11 @@ private String getMemberOfGroupString(String group, String memberOfAttribute) { private String generateGroupSearchFilter(final String groupName, Long domainId) { String groupObjectFilter = "(objectClass=" + - _ldapConfiguration.getGroupObject(domainId) + + LdapConfiguration.getGroupObject(domainId) + ")"; String groupNameFilter = "(" + - _ldapConfiguration.getCommonNameAttribute() + + LdapConfiguration.getCommonNameAttribute() + "=" + (groupName == null ? "*" : LdapUtils.escapeLDAPSearchFilter(groupName)) + ")"; @@ -183,7 +183,7 @@ public LdapUser getUser(final String username, final String type, final String n if("OU".equals(type)) { basedn = name; } else { - basedn = _ldapConfiguration.getBaseDn(domainId); + basedn = LdapConfiguration.getBaseDn(domainId); } final StringBuilder userObjectFilter = getUserObjectFilter(domainId); @@ -227,12 +227,12 @@ public List getUsers(final String username, final LdapContext context, @Override public List getUsersInGroup(String groupName, LdapContext context, Long domainId) throws NamingException { - String attributeName = _ldapConfiguration.getGroupUniqueMemberAttribute(domainId); + String attributeName = LdapConfiguration.getGroupUniqueMemberAttribute(domainId); final SearchControls controls = new SearchControls(); controls.setSearchScope(_ldapConfiguration.getScope()); controls.setReturningAttributes(new String[] {attributeName}); - NamingEnumeration result = context.search(_ldapConfiguration.getBaseDn(domainId), generateGroupSearchFilter(groupName, domainId), controls); + NamingEnumeration result = context.search(LdapConfiguration.getBaseDn(domainId), generateGroupSearchFilter(groupName, domainId), controls); final List users = new ArrayList<>(); //Expecting only one result which has all the users @@ -260,7 +260,7 @@ private LdapUser getUserForDn(String userdn, LdapContext context, Long domainId) controls.setSearchScope(_ldapConfiguration.getScope()); controls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId)); - NamingEnumeration result = context.search(userdn, "(objectClass=" + _ldapConfiguration.getUserObject(domainId) + ")", controls); + NamingEnumeration result = context.search(userdn, "(objectClass=" + LdapConfiguration.getUserObject(domainId) + ")", controls); if (result.hasMoreElements()) { return createUser(result.nextElement(), domainId); } else { @@ -306,12 +306,12 @@ public List searchUsers(final String username, final LdapContext conte searchControls.setSearchScope(_ldapConfiguration.getScope()); searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId)); - String basedn = _ldapConfiguration.getBaseDn(domainId); + String basedn = LdapConfiguration.getBaseDn(domainId); if (StringUtils.isBlank(basedn)) { throw new IllegalArgumentException(String.format("ldap basedn is not configured (for domain: %s)", domainId)); } byte[] cookie = null; - int pageSize = _ldapConfiguration.getLdapPageSize(domainId); + int pageSize = LdapConfiguration.getLdapPageSize(domainId); context.setRequestControls(new Control[]{new PagedResultsControl(pageSize, Control.NONCRITICAL)}); final List users = new ArrayList<>(); NamingEnumeration results; diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDao.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDao.java index 889efa3ef90b..e757def83cef 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDao.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDao.java @@ -29,16 +29,14 @@ public interface LdapConfigurationDao extends GenericDao { /** * @deprecated there might well be more then one ldap implementation on a host and or a double binding of several domains - * @param hostname - * @return + * @param hostname the hostname or ip address of the LDAP server + * @return the LDAP configuration for the given hostname */ @Deprecated LdapConfigurationVO findByHostname(String hostname); LdapConfigurationVO find(String hostname, int port, Long domainId); - LdapConfigurationVO find(String hostname, int port, Long domainId, boolean listAll); - Pair, Integer> searchConfigurations(String hostname, int port, Long domainId); Pair, Integer> searchConfigurations(Long id, String hostname, int port, Long domainId, boolean listAll); diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDaoImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDaoImpl.java index 67d09feed90d..1ce52b9fd0d9 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDaoImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/dao/LdapConfigurationDaoImpl.java @@ -68,12 +68,6 @@ public LdapConfigurationVO find(String hostname, int port, Long domainId) { return findOneBy(sc); } - @Override - public LdapConfigurationVO find(String hostname, int port, Long domainId, boolean listAll) { - SearchCriteria sc = getSearchCriteria(null, hostname, port, domainId, listAll); - return findOneBy(sc); - } - @Override public Pair, Integer> searchConfigurations(final String hostname, final int port, final Long domainId) { SearchCriteria sc = getSearchCriteria(null, hostname, port, domainId, false); diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LdapListUsersCmdTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LdapListUsersCmdTest.java index bd55926e8e00..6af40c5df403 100644 --- a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LdapListUsersCmdTest.java +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LdapListUsersCmdTest.java @@ -167,7 +167,7 @@ public void isACloudstackUser() { LdapUser ldapUser = new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false, null); - boolean result = ldapListUsersCmd.isACloudstackUser(ldapUser); + boolean result = ldapListUsersCmd.isACloudStackUser(ldapUser); assertTrue(result); } @@ -183,7 +183,7 @@ public void isNotACloudstackUser() { LdapUser ldapUser = new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false, null); - boolean result = ldapListUsersCmd.isACloudstackUser(ldapUser); + boolean result = ldapListUsersCmd.isACloudStackUser(ldapUser); assertFalse(result); } diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmdTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmdTest.java index adf0f98f2943..e616bd573789 100644 --- a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmdTest.java +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/api/command/LinkAccountToLdapCmdTest.java @@ -98,5 +98,6 @@ public void execute() throws Exception { assertEquals("type", type, result.getType()); assertEquals("name", ldapDomain, result.getLdapDomain()); assertEquals("accountId", String.valueOf(accountId), result.getAdminId()); + assertEquals("accountName", String.valueOf(accountName), result.getAccountName()); } } diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java index f2ac1dffaf95..6e9944599e56 100644 --- a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java @@ -16,17 +16,21 @@ // under the License. package org.apache.cloudstack.ldap; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import javax.naming.directory.SearchControls; import javax.naming.ldap.LdapContext; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.when; @@ -35,40 +39,46 @@ public class ADLdapUserManagerImplTest { ADLdapUserManagerImpl adLdapUserManager; + MockedStatic LdapConfiguration; @Mock - LdapConfiguration ldapConfiguration; + LdapConfiguration ldapConfigurationMock; @Before - public void init() throws Exception { + public void init() { + LdapConfiguration = Mockito.mockStatic(LdapConfiguration.class,Mockito.CALLS_REAL_METHODS); adLdapUserManager = new ADLdapUserManagerImpl(); - adLdapUserManager._ldapConfiguration = ldapConfiguration; + adLdapUserManager._ldapConfiguration = ldapConfigurationMock; } + @After + public void afterEach() { + LdapConfiguration.close(); + } @Test public void testGenerateADSearchFilterWithNestedGroupsEnabled() { - when(ldapConfiguration.getUserObject(any())).thenReturn("user"); - when(ldapConfiguration.getCommonNameAttribute()).thenReturn("CN"); - when(ldapConfiguration.getBaseDn(any())).thenReturn("DC=cloud,DC=citrix,DC=com"); - when(ldapConfiguration.isNestedGroupsEnabled(any())).thenReturn(true); + when(adLdapUserManager._ldapConfiguration.getUserObject(anyLong())).thenReturn("user"); + when(adLdapUserManager._ldapConfiguration.getCommonNameAttribute()).thenReturn("CN"); + when(adLdapUserManager._ldapConfiguration.getBaseDn(any())).thenReturn("DC=cloud,DC=citrix,DC=com"); + when(adLdapUserManager._ldapConfiguration.isNestedGroupsEnabled(anyLong())).thenReturn(true); String [] groups = {"dev", "dev-hyd"}; for (String group: groups) { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); - assertTrue(("(&(&(objectCategory=person)(objectClass=user))(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); + assertEquals(("(&(&(objectCategory=person)(objectClass=user))(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))"), result); } } @Test public void testGenerateADSearchFilterWithNestedGroupsDisabled() { - when(ldapConfiguration.getUserObject(any())).thenReturn("user"); - when(ldapConfiguration.getCommonNameAttribute()).thenReturn("CN"); - when(ldapConfiguration.getBaseDn(any())).thenReturn("DC=cloud,DC=citrix,DC=com"); - when(ldapConfiguration.isNestedGroupsEnabled(any())).thenReturn(false); + when(adLdapUserManager._ldapConfiguration.getUserObject(anyLong())).thenReturn("user"); + when(adLdapUserManager._ldapConfiguration.getCommonNameAttribute()).thenReturn("CN"); + when(adLdapUserManager._ldapConfiguration.getBaseDn(anyLong())).thenReturn("DC=cloud,DC=citrix,DC=com"); + when(adLdapUserManager._ldapConfiguration.isNestedGroupsEnabled(anyLong())).thenReturn(false); String [] groups = {"dev", "dev-hyd"}; for (String group: groups) { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); - assertTrue(("(&(&(objectCategory=person)(objectClass=user))(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); + assertEquals(("(&(&(objectCategory=person)(objectClass=user))(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))"), result); } } @@ -78,9 +88,9 @@ public void testGenerateADSearchFilterWithNestedGroupsDisabled() { @Test(expected = IllegalArgumentException.class) public void testGetUsersInGroupUsingNullGroup() throws Exception { String[] returnAttributes = {"username", "firstname", "lastname", "email"}; - lenient().when(ldapConfiguration.getScope()).thenReturn(SearchControls.SUBTREE_SCOPE); - lenient().when(ldapConfiguration.getReturnAttributes(null)).thenReturn(returnAttributes); - lenient().when(ldapConfiguration.getBaseDn(any())).thenReturn(null).thenReturn(null).thenReturn("DC=cloud,DC=citrix,DC=com"); + lenient().when(adLdapUserManager._ldapConfiguration.getScope()).thenReturn(SearchControls.SUBTREE_SCOPE); + lenient().when(adLdapUserManager._ldapConfiguration.getReturnAttributes(null)).thenReturn(returnAttributes); + lenient().when(adLdapUserManager._ldapConfiguration.getBaseDn(any())).thenReturn(null).thenReturn(null).thenReturn("DC=cloud,DC=citrix,DC=com"); LdapContext context = ldapContext; String [] groups = {null, "group", null};