[GitHub] [cloudstack] shwstppr commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

[GitBox <gi...@apache.org>]

shwstppr commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r584468447



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -234,22 +269,23 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
 
         SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();
 
-        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
+        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !cmd.isRecursive()) {

Review comment:
       @Spaceman1984 is this change needed?

##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,45 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //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;
             }
             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();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);
+            Domain domain = _domainDao.findById(caller.getDomainId());
+            _accountService.checkAccess(account, domain);

Review comment:
       Not sure if this check is correct. Probably what you want to check is if the account belongs to the same domain or subdomain as the caller but this is checking opposite.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org