Skip to content

cleanup LDAP code according to warnings #11436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+287 −355
Open

cleanup LDAP code according to warnings #11436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...icators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapAddConfigurationCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
13 changes: 6 additions & 7 deletions ...enticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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");
}
}
Expand All @@ -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");
}
Expand All @@ -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;
}
}
4 changes: 2 additions & 2 deletions ...tors/ldap/src/main/java/org/apache/cloudstack/api/command/LdapDeleteConfigurationCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ public class LdapDeleteConfigurationCmd extends BaseCmd {
@Parameter(name = ApiConstants.HOST_NAME, type = CommandType.STRING, required = true, 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() {
Expand Down
31 changes: 15 additions & 16 deletions ...thenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapImportUsersCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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 an 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 {
Expand Down Expand Up @@ -145,21 +144,21 @@ public void execute()
users = _ldapManager.getUsers(domainId);
}
} catch (NoLdapUserMatchingQueryException ex) {
users = new ArrayList<LdapUser>();
logger.info("No Ldap user matching query. " + " ::: " + ex.getMessage());
users = new ArrayList<>();
logger.info("No Ldap user matching query. ::: {}", ex.getMessage());
}

List<LdapUser> addedUsers = new ArrayList<LdapUser>();
List<LdapUser> 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<LdapUserResponse> response = new ListResponse<LdapUserResponse>();
ListResponse<LdapUserResponse> response = new ListResponse<>();
response.setResponses(createLdapUserResponse(addedUsers));
response.setResponseName(getCommandName());
setResponseObject(response);
Expand All @@ -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() {
Expand Down Expand Up @@ -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 either 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)
Expand All @@ -225,7 +224,7 @@ private Domain getDomain(LdapUser user) {
}

private List<LdapUserResponse> createLdapUserResponse(List<LdapUser> users) {
final List<LdapUserResponse> ldapResponses = new ArrayList<LdapUserResponse>();
final List<LdapUserResponse> ldapResponses = new ArrayList<>();
for (final LdapUser user : users) {
final LdapUserResponse ldapResponse = _ldapManager.createLdapUserResponse(user);
ldapResponse.setObjectName("LdapUser");
Expand All @@ -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");
}
}
Expand Down
16 changes: 9 additions & 7 deletions ...cators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListConfigurationCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,19 @@ 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.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() {
Expand All @@ -67,7 +69,7 @@ public LdapListConfigurationCmd(final LdapManager ldapManager) {
}

private List<LdapConfigurationResponse> createLdapConfigurationResponses(final List<? extends LdapConfigurationVO> configurations) {
final List<LdapConfigurationResponse> responses = new ArrayList<LdapConfigurationResponse>();
final List<LdapConfigurationResponse> responses = new ArrayList<>();
for (final LdapConfigurationVO resource : configurations) {
final LdapConfigurationResponse configurationResponse = _ldapManager.createLdapConfigurationResponse(resource);
configurationResponse.setObjectName("LdapConfiguration");
Expand All @@ -80,7 +82,7 @@ private List<LdapConfigurationResponse> createLdapConfigurationResponses(final L
public void execute() {
final Pair<List<? extends LdapConfigurationVO>, Integer> result = _ldapManager.listConfigurations(this);
final List<LdapConfigurationResponse> responses = createLdapConfigurationResponses(result.first());
final ListResponse<LdapConfigurationResponse> response = new ListResponse<LdapConfigurationResponse>();
final ListResponse<LdapConfigurationResponse> response = new ListResponse<>();
response.setResponses(responses, result.second());
response.setResponseName(getCommandName());
setResponseObject(response);
Expand Down
Loading
Loading