[4.19] server, api, ui: access improvements and assorted fixes (#22)

* server, api, ui: access improvements and assorted fixes

Fixes domain-admin access check to prevent unauthorized access.

Co-authored-by: Fabricio Duarte <[email protected]>
Co-authored-by: nvazquez <[email protected]>
Co-authored-by: Abhishek Kumar <[email protected]>

* Revert "server: refactor listNetworks api database retrievals (#9184)"

This reverts commit c7f1ba5.

* Fix snapshot chain being deleted on XenServer (#9447)

Using XenServer as the hypervisor, when deleting a snapshot that has a parent, that parent will also get erased on storage, causing data loss. This behavior was introduced with #7873, where the list of snapshot states that can be deleted was changed to add BackedUp snapshots.

This PR changes the states list back to the original list, and swaps the while loop for a do while loop to account for the changes in #7873.

Fixes #9446

* UI: Display Firewall, LB and Port Forwading rules tab for CKS clusters deployed on isolated networks (#9458)

---------

Co-authored-by: nvazquez <[email protected]>
Co-authored-by: Fabricio Duarte <[email protected]>
Co-authored-by: João Jandre <[email protected]>
Co-authored-by: Pearl Dsilva <[email protected]>

.github/workflows/ci.yml

@@ -35,6 +35,7 @@ jobs:
       fail-fast: false
       matrix:
         tests: [ "smoke/test_accounts
+                  smoke/test_account_access
                   smoke/test_affinity_groups
                   smoke/test_affinity_groups_projects
                   smoke/test_annotations

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java

@@ -102,6 +102,8 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
     @Inject
     SnapshotZoneDao snapshotZoneDao;
 
+    private final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error);
+
     public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
         List<SnapshotDataStoreVO> snaps = snapshotStoreDao.listReadyBySnapshot(snapshotId, DataStoreRole.Image);
         for (SnapshotDataStoreVO ref : snaps) {
@@ -199,9 +201,8 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToStr
 
         boolean result = false;
         boolean resultIsSet = false;
-        final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.BackedUp, Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error);
         try {
-            while (snapshot != null && snapshotStatesAbleToDeleteSnapshot.contains(snapshot.getState())) {
+            do {
                 SnapshotInfo child = snapshot.getChild();
 
                 if (child != null) {
@@ -247,7 +248,7 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot, String storageToStr
                 }
 
                 snapshot = parent;
-            }
+            } while (snapshot != null && snapshotStatesAbleToDeleteSnapshot.contains(snapshot.getState()));
         } catch (Exception e) {
             s_logger.error(String.format("Failed to delete snapshot [%s] on storage [%s] due to [%s].", snapshotTo, storageToString, e.getMessage()), e);
         }

server/src/main/java/com/cloud/acl/DomainChecker.java

@@ -208,38 +208,66 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
 
             return true;
         } else if (entity instanceof Network && accessType != null && accessType == AccessType.UseEntry) {
-            _networkMgr.checkNetworkPermissions(caller, (Network)entity);
+            _networkMgr.checkNetworkPermissions(caller, (Network) entity);
         } else if (entity instanceof Network && accessType != null && accessType == AccessType.OperateEntry) {
             _networkMgr.checkNetworkOperatePermissions(caller, (Network)entity);
         } else if (entity instanceof VirtualRouter) {
             _networkMgr.checkRouterPermissions(caller, (VirtualRouter)entity);
         } else if (entity instanceof AffinityGroup) {
             return false;
         } else {
-            if (_accountService.isNormalUser(caller.getId())) {
-                Account account = _accountDao.findById(entity.getAccountId());
-                String errorMessage = String.format("%s does not have permission to operate with resource", caller);
-                if (account != null && account.getType() == Account.Type.PROJECT) {
-                    //only project owner can delete/modify the project
-                    if (accessType != null && accessType == AccessType.ModifyProject) {
-                        if (!_projectMgr.canModifyProjectAccount(caller, account.getId())) {
-                            throw new PermissionDeniedException(errorMessage);
-                        }
-                    } else if (!_projectMgr.canAccessProjectAccount(caller, account.getId())) {
-                        throw new PermissionDeniedException(errorMessage);
-                    }
-                    checkOperationPermitted(caller, entity);
-                } else {
-                    if (caller.getId() != entity.getAccountId()) {
-                        throw new PermissionDeniedException(errorMessage);
-                    }
+            validateCallerHasAccessToEntityOwner(caller, entity, accessType);
+        }
+        return true;
+    }
+
+    protected void validateCallerHasAccessToEntityOwner(Account caller, ControlledEntity entity, AccessType accessType) {
+        PermissionDeniedException exception = new PermissionDeniedException("Caller does not have permission to operate with provided resource.");
+        String entityLog = String.format("entity [owner ID: %d, type: %s]", entity.getAccountId(),
+                entity.getEntityType().getSimpleName());
+
+        if (_accountService.isRootAdmin(caller.getId())) {
+            return;
+        }
+
+        if (caller.getId() == entity.getAccountId()) {
+            return;
+        }
+
+        Account owner = _accountDao.findById(entity.getAccountId());
+        if (owner == null) {
+            s_logger.error(String.format("Owner not found for %s", entityLog));
+            throw exception;
+        }
+
+        Account.Type callerAccountType = caller.getType();
+        if ((callerAccountType == Account.Type.DOMAIN_ADMIN || callerAccountType == Account.Type.RESOURCE_DOMAIN_ADMIN) &&
+                _domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+            return;
+        }
+
+        if (owner.getType() == Account.Type.PROJECT) {
+            // only project owner can delete/modify the project
+            if (accessType == AccessType.ModifyProject) {
+                if (!_projectMgr.canModifyProjectAccount(caller, owner.getId())) {
+                    s_logger.error(String.format("Caller ID: %d does not have permission to modify project with " +
+                            "owner ID: %d", caller.getId(), owner.getId()));
+                    throw exception;
                 }
+            } else if (!_projectMgr.canAccessProjectAccount(caller, owner.getId())) {
+                s_logger.error(String.format("Caller ID: %d does not have permission to access project with " +
+                        "owner ID: %d", caller.getId(), owner.getId()));
+                throw exception;
             }
+            checkOperationPermitted(caller, entity);
+            return;
         }
-        return true;
+
+        s_logger.error(String.format("Caller ID: %d does not have permission to access %s", caller.getId(), entityLog));
+        throw exception;
     }
 
-    private boolean checkOperationPermitted(Account caller, ControlledEntity entity) {
+    protected boolean checkOperationPermitted(Account caller, ControlledEntity entity) {
         User user = CallContext.current().getCallingUser();
         Project project = projectDao.findByProjectAccountId(entity.getAccountId());
         if (project == null) {