Merge fc10728f10 into bce3e54a7e

api/src/main/java/org/apache/cloudstack/acl/APIChecker.java

@@ -19,6 +19,7 @@ package org.apache.cloudstack.acl;
 import com.cloud.exception.PermissionDeniedException;
 import com.cloud.user.Account;
 import com.cloud.user.User;
+import com.cloud.utils.Pair;
 import com.cloud.utils.component.Adapter;
 
 import java.util.List;
@@ -43,4 +44,7 @@ public interface APIChecker extends Adapter {
      */
     List<String> getApisAllowedToUser(Role role, User user, List<String> apiNames) throws PermissionDeniedException;
     boolean isEnabled();
+
+    default Pair<Role, List<RolePermission>> getRolePermissions(long roleId) { return null; }
+    default boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions) { return false; }
 }

plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java

@@ -107,7 +107,8 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
         return accountService.getAccount(accountId);
     }
 
-    protected Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
+    @Override
+    public Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
         final Role accountRole = roleService.findRole(roleId);
         if (accountRole == null || accountRole.getId() < 1L) {
             return new Pair<>(null, null);
@@ -149,7 +150,7 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
             throw new PermissionDeniedException(String.format("Account role for user id [%s] cannot be found.", user.getUuid()));
         }
         if (accountRole.getRoleType() == RoleType.Admin && accountRole.getId() == RoleType.Admin.getId()) {
-            logger.info("Account for user id {} is Root Admin or Domain Admin, all APIs are allowed.", user.getUuid());
+            logger.info("Account for user id {} is Root Admin, all APIs are allowed.", user.getUuid());
             return true;
         }
         List<RolePermission> allPermissions = roleAndPermissions.second();
@@ -180,6 +181,25 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
         throw new UnavailableCommandException(String.format("The API [%s] does not exist or is not available for the account %s.", commandName, account));
     }
 
+    @Override
+    public boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions) {
+        if (accountRole == null) {
+            throw new PermissionDeniedException(String.format("The account [%s] has role null or unknown.", account));
+        }
+
+        if (accountRole.getRoleType() == RoleType.Admin && accountRole.getId() == RoleType.Admin.getId()) {
+            if (logger.isTraceEnabled()) {
+                logger.trace(String.format("Account [%s] is Root Admin, all APIs are allowed.", account));
+            }
+            return true;
+        }
+
+        if (checkApiPermissionByRole(accountRole, commandName, allPermissions)) {
+            return true;
+        }
+        throw new UnavailableCommandException(String.format("The API [%s] does not exist or is not available for the account %s.", commandName, account));
+    }
+
     /**
      * Only one strategy should be used between StaticRoleBasedAPIAccessChecker and DynamicRoleBasedAPIAccessChecker
      * Default behavior is to use the Dynamic version. The StaticRoleBasedAPIAccessChecker is the legacy version.

server/src/main/java/com/cloud/user/AccountManagerImpl.java

@@ -47,6 +47,7 @@ import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.InfrastructureEntity;
 import org.apache.cloudstack.acl.QuerySelector;
 import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.acl.RolePermission;
 import org.apache.cloudstack.acl.RoleService;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.acl.SecurityChecker;
@@ -1438,29 +1439,35 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                     requested.getUuid(),
                     requested.getRoleId()));
         }
+        if (caller.getRoleId().equals(requested.getRoleId())) {
+            return;
+        }
         List<APIChecker> apiCheckers = getEnabledApiCheckers();
+        for (APIChecker apiChecker : apiCheckers) {
+            checkApiAccess(apiChecker, caller, requested);
+        }
+    }
+
+    private void checkApiAccess(APIChecker apiChecker, Account caller, Account requested) throws PermissionDeniedException {
+        Pair<Role, List<RolePermission>> roleAndPermissionsForCaller = apiChecker.getRolePermissions(caller.getRoleId());
+        Pair<Role, List<RolePermission>> roleAndPermissionsForRequested = apiChecker.getRolePermissions(requested.getRoleId());
         for (String command : apiNameList) {
             try {
-                checkApiAccess(apiCheckers, requested, command);
+                if (roleAndPermissionsForRequested == null) {
-            } catch (PermissionDeniedException pde) {
+                    apiChecker.checkAccess(caller, command);
-                if (logger.isTraceEnabled()) {
+                } else {
-                    logger.trace(String.format(
+                    apiChecker.checkAccess(caller, command, roleAndPermissionsForRequested.first(), roleAndPermissionsForRequested.second());
-                            "Checking for permission to \"%s\" is irrelevant as it is not requested for %s [%s]",
-                            command,
-                            requested.getAccountName(),
-                            requested.getUuid()
-                        )
-                    );
                 }
+            } catch (PermissionDeniedException pde) {
                 continue;
             }
             // so requested can, now make sure caller can as well
             try {
-                if (logger.isTraceEnabled()) {
+                if (roleAndPermissionsForCaller == null) {
-                    logger.trace(String.format("permission to \"%s\" is requested",
+                    apiChecker.checkAccess(caller, command);
-                            command));
+                } else {
+                    apiChecker.checkAccess(caller, command, roleAndPermissionsForCaller.first(), roleAndPermissionsForCaller.second());
                 }
-                checkApiAccess(apiCheckers, caller, command);
             } catch (PermissionDeniedException pde) {
                 String msg = String.format("User of Account %s and domain %s can not create an account with access to more privileges they have themself.",
                         caller, _domainMgr.getDomain(caller.getDomainId()));