Skip to content

small refactors on account manager #11784

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
+20 −21
Open

small refactors on account manager #11784

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
41 changes: 20 additions & 21 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;

Expand Down Expand Up @@ -175,6 +174,7 @@
import com.cloud.utils.ConstantTimeComparator;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.StringUtils;
import com.cloud.utils.Ternary;
import com.cloud.utils.component.ComponentContext;
import com.cloud.utils.component.Manager;
Expand Down Expand Up @@ -587,10 +587,9 @@ public boolean isAdmin(Long accountId) {
}
if ((isRootAdmin(accountId)) || (isDomainAdmin(accountId)) || (isResourceDomainAdmin(accountId))) {
return true;
} else if (acct.getType() == Account.Type.READ_ONLY_ADMIN) {
return true;
} else {
return acct.getType() == Account.Type.READ_ONLY_ADMIN;
}

}
return false;
}
Expand Down Expand Up @@ -644,10 +643,7 @@ public boolean isDomainAdmin(Long accountId) {
@Override
public boolean isNormalUser(long accountId) {
AccountVO acct = _accountDao.findById(accountId);
if (acct != null && acct.getType() == Account.Type.NORMAL) {
return true;
}
return false;
return acct != null && acct.getType() == Account.Type.NORMAL;
}

@Override
Expand Down Expand Up @@ -678,10 +674,7 @@ public boolean isInternalAccount(long accountId) {
if (account == null) {
return false; //account is deleted or does not exist
}
if (isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN)) {
return true;
}
return false;
return isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN);
}

@Override
Expand Down Expand Up @@ -731,12 +724,7 @@ public void checkAccess(Account caller, AccessType accessType, boolean sameOwner
HashMap<Long, List<ControlledEntity>> domains = new HashMap<>();

for (ControlledEntity entity : entities) {
long domainId = entity.getDomainId();
if (entity.getAccountId() != -1 && domainId == -1) { // If account exists domainId should too so calculate
// it. This condition might be hit for templates or entities which miss domainId in their tables
Account account = ApiDBUtils.findAccountById(entity.getAccountId());
domainId = account != null ? account.getDomainId() : -1;
}
long domainId = getDomainIdFor(entity);
if (entity.getAccountId() != -1 && domainId != -1 && !(entity instanceof VirtualMachineTemplate)
&& !(entity instanceof Network && accessType != null && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry))
&& !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter)) {
Expand Down Expand Up @@ -788,6 +776,17 @@ public void checkAccess(Account caller, AccessType accessType, boolean sameOwner

}

private static long getDomainIdFor(ControlledEntity entity) {
long domainId = entity.getDomainId();
if (entity.getAccountId() != -1 && domainId == -1) {
// If account exists domainId should too so calculate it.
// This condition might be hit for templates or entities which miss domainId in their tables
Account account = ApiDBUtils.findAccountById(entity.getAccountId());
domainId = account != null ? account.getDomainId() : -1;
}
return domainId;
}

@Override
public void validateAccountHasAccessToResource(Account account, AccessType accessType, Object resource) {
Class<?> resourceClass = resource.getClass();
Expand Down Expand Up @@ -2813,11 +2812,11 @@ public UserAccount authenticateUser(final String username, final String password
final Boolean ApiSourceCidrChecksEnabled = ApiServiceConfiguration.ApiSourceCidrChecksEnabled.value();

if (ApiSourceCidrChecksEnabled) {
logger.debug("CIDRs from which account '" + account.toString() + "' is allowed to perform API calls: " + accessAllowedCidrs);
logger.debug("CIDRs from which account '{}' is allowed to perform API calls: {}", account, accessAllowedCidrs);

// Block when is not in the list of allowed IPs
if (!NetUtils.isIpInCidrList(loginIpAddress, accessAllowedCidrs.split(","))) {
logger.warn("Request by account '" + account.toString() + "' was denied since " + loginIpAddress.toString().replace("/", "") + " does not match " + accessAllowedCidrs);
logger.warn("Request by account '{}' was denied since {} does not match {}", account , loginIpAddress.toString().replace("/", ""), accessAllowedCidrs);
throw new CloudAuthenticationException("Failed to authenticate user '" + username + "' in domain '" + domain.getPath() + "' from ip "
+ loginIpAddress.toString().replace("/", "") + "; please provide valid credentials");
}
Expand Down Expand Up @@ -2990,7 +2989,7 @@ private UserAccount getUserAccountForSSO(String username, Long domainId, Map<Str
if (unsignedRequestBuffer.length() != 0) {
unsignedRequestBuffer.append("&");
}
unsignedRequestBuffer.append(paramName).append("=").append(URLEncoder.encode(paramValue, "UTF-8"));
unsignedRequestBuffer.append(paramName).append("=").append(URLEncoder.encode(paramValue, StringUtils.getPreferredCharset()));
}
}

Expand Down
Loading