Skip to content
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

Jump to bottom

Added recursive fetch of child domains for listUsageRecords API call #4717

Merged
merged 10 commits into from Apr 10, 2021
Merged

Added recursive fetch of child domains for listUsageRecords API call #4717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0da721a
Added recursive fetch of domains for listUsageRecords API call
Spaceman1984 Feb 22, 2021
e66d92b
Added security check for domain admin
Spaceman1984 Feb 22, 2021
7a70230
Review changes
Spaceman1984 Feb 26, 2021
9270df4
Changed account check
Spaceman1984 Mar 1, 2021
5d43c1c
Removed unused var
Spaceman1984 Mar 1, 2021
71c7c71
Adding recursive check back
Spaceman1984 Mar 1, 2021
19fd612
Changed logic around setting the account id
Spaceman1984 Mar 1, 2021
a45fcee
Fixed null account issue
Spaceman1984 Mar 16, 2021
83c9b0a
Refactored code
Spaceman1984 Mar 17, 2021
80860ee
Added message regarding account id
Spaceman1984 Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageRecordsCmd.java
View file
Open in desktop
Expand Up @@ -85,6 +85,11 @@ public class ListUsageRecordsCmd extends BaseListCmd {
@Parameter(name = ApiConstants.OLD_FORMAT, type = CommandType.BOOLEAN, description = "Flag to enable description rendered in old format which uses internal database IDs instead of UUIDs. False by default.")
private Boolean oldFormat;

@Parameter(name = ApiConstants.IS_RECURSIVE, type = CommandType.BOOLEAN,
description = "Specify if usage records should be fetched recursively per domain. If an account id is passed, records will be limited to that account.",
since = "4.15")
private Boolean recursive = false;

/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
Expand Down Expand Up @@ -153,6 +158,10 @@ public boolean getOldFormat() {
return oldFormat != null && oldFormat;
}

public Boolean isRecursive() {
return recursive;
}

/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
Expand Down
105 changes: 85 additions & 20 deletions server/src/main/java/com/cloud/usage/UsageServiceImpl.java
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.domain.Domain;
import org.apache.cloudstack.api.command.admin.usage.GenerateUsageRecordsCmd;
import org.apache.cloudstack.api.command.admin.usage.ListUsageRecordsCmd;
import org.apache.cloudstack.api.command.admin.usage.RemoveRawUsageRecordsCmd;
Expand Down Expand Up @@ -200,22 +201,41 @@ public Pair<List<? extends Usage>, Integer> getUsageRecords(ListUsageRecordsCmd
}
}

boolean isAdmin = false;
boolean isDomainAdmin = false;
boolean ignoreAccountId = false;
boolean isDomainAdmin = _accountService.isDomainAdmin(caller.getId());
boolean isNormalUser = _accountService.isNormalUser(caller.getId());

//If accountId couldn't be found using accountName and domainId, get it from userContext
if (accountId == null) {
accountId = caller.getId();
//List records for all the accounts if the caller account is of type admin.
//If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
if (_accountService.isRootAdmin(caller.getId())) {
isAdmin = true;
} else if (_accountService.isDomainAdmin(caller.getId())) {
isDomainAdmin = true;
}
ignoreAccountId = _accountService.isRootAdmin(caller.getId());
s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
}

// Check if a domain admin is allowed to access the requested domain id
if (isDomainAdmin) {
if (domainId != null) {
Account callerAccount = _accountService.getAccount(caller.getId());
Domain domain = _domainDao.findById(domainId);
_accountService.checkAccess(callerAccount, domain);
} else {
// Domain admins can only access their own domain's usage records.
// Set the domain if not specified.
domainId = caller.getDomainId();
}

if (cmd.getAccountId() != null) {
// Check if a domain admin is allowed to access the requested account info.
checkDomainAdminAccountAccess(accountId, domainId);
}
}

Spaceman1984 marked this conversation as resolved.
Show resolved Hide resolved
// By default users do not have access to this API.
// Adding checks here in case someone changes the default access.
checkUserAccess(cmd, accountId, caller, isNormalUser);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Spaceman1984 is this method call required for domain admin ?


Date startDate = cmd.getStartDate();
Date endDate = cmd.getEndDate();
if (startDate.after(endDate)) {
Expand All @@ -234,22 +254,27 @@ public Pair<List<? extends Usage>, Integer> getUsageRecords(ListUsageRecordsCmd

SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();

if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
}

if (isDomainAdmin) {
SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
List<DomainVO> domains = _domainDao.search(sdc, null);
List<Long> domainIds = new ArrayList<Long>();
for (DomainVO domain : domains)
domainIds.add(domain.getId());
sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !ignoreAccountId) {
// account exists and either domain on user role
// If not recursive and the account belongs to the user/domain admin, or the account was passed in, filter
if ((accountId == caller.getId() && !cmd.isRecursive()) || cmd.getAccountId() != null){
sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
}
}

if (domainId != null) {
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
if (cmd.isRecursive()) {
SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
List<DomainVO> domains = _domainDao.search(sdc, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify, reuse existing DomainDao::findAllChildren

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhtyd Using findAllChildren here would not pass the top domain.

List<Long> domainIds = new ArrayList<Long>();
for (DomainVO domain : domains) {
domainIds.add(domain.getId());
}
sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
} else {
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
}
}

if (usageType != null) {
Expand Down Expand Up @@ -372,6 +397,46 @@ public Pair<List<? extends Usage>, Integer> getUsageRecords(ListUsageRecordsCmd
return new Pair<List<? extends Usage>, Integer>(usageRecords.first(), usageRecords.second());
}

private void checkUserAccess(ListUsageRecordsCmd cmd, Long accountId, Account caller, boolean isNormalUser) {
if (isNormalUser) {
// A user can only access their own account records
if (caller.getId() != accountId) {
throw new PermissionDeniedException("Users are only allowed to list usage records for their own account.");
}
// Users cannot get recursive records
if (cmd.isRecursive()) {
throw new PermissionDeniedException("Users are not allowed to list usage records recursively.");
}
// Users cannot get domain records
if (cmd.getDomainId() != null) {
throw new PermissionDeniedException("Users are not allowed to list usage records for a domain");
}
}
}

private void checkDomainAdminAccountAccess(Long accountId, Long domainId) {
Account account = _accountService.getAccount(accountId);
boolean matchFound = false;

if (account.getDomainId() == domainId) {
matchFound = true;
} else {

// Check if the account is in a child domain of this domain admin.
List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(domainId).getPath(), domainId);

for (DomainVO domainVO : childDomains) {
if (account.getDomainId() == domainVO.getId()) {
matchFound = true;
break;
}
}
}
if (!matchFound) {
throw new PermissionDeniedException("Domain admins may only retrieve usage records for accounts in their own domain and child domains.");
}
}

@Override
public TimeZone getUsageTimezone() {
return _usageTimezone;
Expand Down