Skip to content

Commit 99a9063

Browse files
authored
server: Added recursive fetch of child domains for listUsageRecords API call (#4717)
When calling the listUasageRecords API records per domain are fetched recursively. This is not the case if you specify a domain id. This PR adds a new parameter to enable fetching records recursively (isRecursive) when passing the domain id.
1 parent 03c05bc commit 99a9063

2 files changed

Lines changed: 94 additions & 20 deletions

File tree

api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageRecordsCmd.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public class ListUsageRecordsCmd extends BaseListCmd {
8585
@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.")
8686
private Boolean oldFormat;
8787

88+
@Parameter(name = ApiConstants.IS_RECURSIVE, type = CommandType.BOOLEAN,
89+
description = "Specify if usage records should be fetched recursively per domain. If an account id is passed, records will be limited to that account.",
90+
since = "4.15")
91+
private Boolean recursive = false;
92+
8893
/////////////////////////////////////////////////////
8994
/////////////////// Accessors ///////////////////////
9095
/////////////////////////////////////////////////////
@@ -153,6 +158,10 @@ public boolean getOldFormat() {
153158
return oldFormat != null && oldFormat;
154159
}
155160

161+
public Boolean isRecursive() {
162+
return recursive;
163+
}
164+
156165
/////////////////////////////////////////////////////
157166
/////////////// API Implementation///////////////////
158167
/////////////////////////////////////////////////////

server/src/main/java/com/cloud/usage/UsageServiceImpl.java

Lines changed: 85 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import javax.inject.Inject;
2727
import javax.naming.ConfigurationException;
2828

29+
import com.cloud.domain.Domain;
2930
import org.apache.cloudstack.api.command.admin.usage.GenerateUsageRecordsCmd;
3031
import org.apache.cloudstack.api.command.admin.usage.ListUsageRecordsCmd;
3132
import org.apache.cloudstack.api.command.admin.usage.RemoveRawUsageRecordsCmd;
@@ -200,22 +201,41 @@ public Pair<List<? extends Usage>, Integer> getUsageRecords(ListUsageRecordsCmd
200201
}
201202
}
202203

203-
boolean isAdmin = false;
204-
boolean isDomainAdmin = false;
204+
boolean ignoreAccountId = false;
205+
boolean isDomainAdmin = _accountService.isDomainAdmin(caller.getId());
206+
boolean isNormalUser = _accountService.isNormalUser(caller.getId());
205207

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

217+
// Check if a domain admin is allowed to access the requested domain id
218+
if (isDomainAdmin) {
219+
if (domainId != null) {
220+
Account callerAccount = _accountService.getAccount(caller.getId());
221+
Domain domain = _domainDao.findById(domainId);
222+
_accountService.checkAccess(callerAccount, domain);
223+
} else {
224+
// Domain admins can only access their own domain's usage records.
225+
// Set the domain if not specified.
226+
domainId = caller.getDomainId();
227+
}
228+
229+
if (cmd.getAccountId() != null) {
230+
// Check if a domain admin is allowed to access the requested account info.
231+
checkDomainAdminAccountAccess(accountId, domainId);
232+
}
233+
}
234+
235+
// By default users do not have access to this API.
236+
// Adding checks here in case someone changes the default access.
237+
checkUserAccess(cmd, accountId, caller, isNormalUser);
238+
219239
Date startDate = cmd.getStartDate();
220240
Date endDate = cmd.getEndDate();
221241
if (startDate.after(endDate)) {
@@ -234,22 +254,27 @@ public Pair<List<? extends Usage>, Integer> getUsageRecords(ListUsageRecordsCmd
234254

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

237-
if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
238-
sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
239-
}
240-
241-
if (isDomainAdmin) {
242-
SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
243-
sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
244-
List<DomainVO> domains = _domainDao.search(sdc, null);
245-
List<Long> domainIds = new ArrayList<Long>();
246-
for (DomainVO domain : domains)
247-
domainIds.add(domain.getId());
248-
sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
257+
if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !ignoreAccountId) {
258+
// account exists and either domain on user role
259+
// If not recursive and the account belongs to the user/domain admin, or the account was passed in, filter
260+
if ((accountId == caller.getId() && !cmd.isRecursive()) || cmd.getAccountId() != null){
261+
sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
262+
}
249263
}
250264

251265
if (domainId != null) {
252-
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
266+
if (cmd.isRecursive()) {
267+
SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
268+
sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
269+
List<DomainVO> domains = _domainDao.search(sdc, null);
270+
List<Long> domainIds = new ArrayList<Long>();
271+
for (DomainVO domain : domains) {
272+
domainIds.add(domain.getId());
273+
}
274+
sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
275+
} else {
276+
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
277+
}
253278
}
254279

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

400+
private void checkUserAccess(ListUsageRecordsCmd cmd, Long accountId, Account caller, boolean isNormalUser) {
401+
if (isNormalUser) {
402+
// A user can only access their own account records
403+
if (caller.getId() != accountId) {
404+
throw new PermissionDeniedException("Users are only allowed to list usage records for their own account.");
405+
}
406+
// Users cannot get recursive records
407+
if (cmd.isRecursive()) {
408+
throw new PermissionDeniedException("Users are not allowed to list usage records recursively.");
409+
}
410+
// Users cannot get domain records
411+
if (cmd.getDomainId() != null) {
412+
throw new PermissionDeniedException("Users are not allowed to list usage records for a domain");
413+
}
414+
}
415+
}
416+
417+
private void checkDomainAdminAccountAccess(Long accountId, Long domainId) {
418+
Account account = _accountService.getAccount(accountId);
419+
boolean matchFound = false;
420+
421+
if (account.getDomainId() == domainId) {
422+
matchFound = true;
423+
} else {
424+
425+
// Check if the account is in a child domain of this domain admin.
426+
List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(domainId).getPath(), domainId);
427+
428+
for (DomainVO domainVO : childDomains) {
429+
if (account.getDomainId() == domainVO.getId()) {
430+
matchFound = true;
431+
break;
432+
}
433+
}
434+
}
435+
if (!matchFound) {
436+
throw new PermissionDeniedException("Domain admins may only retrieve usage records for accounts in their own domain and child domains.");
437+
}
438+
}
439+
375440
@Override
376441
public TimeZone getUsageTimezone() {
377442
return _usageTimezone;

0 commit comments

Comments
 (0)