From e5848acdd073492cfcc4b74e2d9bf081791218df Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 4 Jul 2025 14:32:45 +0200 Subject: [PATCH 1/4] server: optimize account creation by pre-loading the role permissions --- .../org/apache/cloudstack/acl/APIChecker.java | 4 +++ .../acl/DynamicRoleBasedAPIAccessChecker.java | 24 +++++++++++-- .../acl/ProjectRoleBasedApiAccessChecker.java | 13 ++++++- .../acl/StaticRoleBasedAPIAccessChecker.java | 10 ++++++ .../ratelimit/ApiRateLimitServiceImpl.java | 12 +++++++ .../com/cloud/user/AccountManagerImpl.java | 35 +++++++++++-------- 6 files changed, 81 insertions(+), 17 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java index 660f64f43ef2..08a80a52d5e3 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java +++ b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java @@ -19,6 +19,7 @@ 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 getApisAllowedToUser(Role role, User user, List apiNames) throws PermissionDeniedException; boolean isEnabled(); + + Pair> getRolePermissions(long roleId); + boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions); } diff --git a/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java b/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java index 030e0bcf0141..8ae1ee19b240 100644 --- a/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java +++ b/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java @@ -107,7 +107,8 @@ protected Account getAccountFromId(long accountId) { return accountService.getAccount(accountId); } - protected Pair> getRolePermissions(long roleId) { + @Override + public Pair> 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 boolean checkAccess(User user, String commandName) throws PermissionDenie 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 allPermissions = roleAndPermissions.second(); @@ -180,6 +181,25 @@ public boolean checkAccess(Account account, String commandName) { 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 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. diff --git a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java index 2e7ae23d6f1b..3b98e736ec41 100644 --- a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java +++ b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java @@ -21,8 +21,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.cloudstack.acl.RolePermissionEntity.Permission; +import org.apache.cloudstack.acl.RolePermissionEntity.Permission; import org.apache.cloudstack.context.CallContext; import com.cloud.exception.PermissionDeniedException; @@ -33,6 +33,7 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; +import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.PluggableService; @@ -195,4 +196,14 @@ public List getServices() { public void setServices(List services) { this.services = services; } + + @Override + public Pair> getRolePermissions(long roleId) { + return null; + } + + @Override + public boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { + return false; + } } diff --git a/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java b/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java index 3444f967d784..cb335a948e2c 100644 --- a/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java +++ b/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java @@ -33,6 +33,7 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; +import com.cloud.utils.Pair; import com.cloud.utils.PropertiesUtil; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.PluggableService; @@ -200,4 +201,13 @@ public void setCommandPropertyFiles(Set commandPropertyFiles) { this.commandPropertyFiles = commandPropertyFiles; } + @Override + public Pair> getRolePermissions(long roleId) { + return null; + } + + @Override + public boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { + return false; + } } diff --git a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java index 917cd7bb2b46..901159a1de73 100644 --- a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java +++ b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java @@ -27,6 +27,7 @@ import net.sf.ehcache.CacheManager; import org.apache.cloudstack.acl.Role; +import org.apache.cloudstack.acl.RolePermission; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.springframework.stereotype.Component; @@ -42,6 +43,7 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; +import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; @Component @@ -256,4 +258,14 @@ public void setEnabled(boolean enabled) { this.enabled = enabled; } + + @Override + public Pair> getRolePermissions(long roleId) { + return null; + } + + @Override + public boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { + return false; + } } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index ecd761bb7d94..b4b8be61b157 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -47,6 +47,7 @@ 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; @@ -1431,29 +1432,35 @@ protected void checkRoleEscalation(Account caller, Account requested) { requested.getUuid(), requested.getRoleId())); } + if (caller.getRoleId().equals(requested.getRoleId())) { + return; + } List apiCheckers = getEnabledApiCheckers(); + for (APIChecker apiChecker : apiCheckers) { + checkApiAccess(apiChecker, caller, requested); + } + } + + private void checkApiAccess(APIChecker apiChecker, Account caller, Account requested) throws PermissionDeniedException { + Pair> roleAndPermissionsForCaller = apiChecker.getRolePermissions(caller.getRoleId()); + Pair> roleAndPermissionsForRequested = apiChecker.getRolePermissions(requested.getRoleId()); for (String command : apiNameList) { try { - checkApiAccess(apiCheckers, requested, command); - } catch (PermissionDeniedException pde) { - if (logger.isTraceEnabled()) { - logger.trace(String.format( - "Checking for permission to \"%s\" is irrelevant as it is not requested for %s [%s]", - command, - requested.getAccountName(), - requested.getUuid() - ) - ); + if (roleAndPermissionsForRequested == null) { + apiChecker.checkAccess(caller, command); + } else { + apiChecker.checkAccess(caller, command, roleAndPermissionsForRequested.first(), roleAndPermissionsForRequested.second()); } + } catch (PermissionDeniedException pde) { continue; } // so requested can, now make sure caller can as well try { - if (logger.isTraceEnabled()) { - logger.trace(String.format("permission to \"%s\" is requested", - command)); + if (roleAndPermissionsForCaller == null) { + apiChecker.checkAccess(caller, 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())); From fc10728f10f3bb06e3f412eaf60e9379e6ff892e Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 18 Aug 2025 17:17:31 +0200 Subject: [PATCH 2/4] APIChecker: add default implementation for methods --- .../java/org/apache/cloudstack/acl/APIChecker.java | 4 ++-- .../acl/ProjectRoleBasedApiAccessChecker.java | 13 +------------ .../acl/StaticRoleBasedAPIAccessChecker.java | 10 ---------- .../ratelimit/ApiRateLimitServiceImpl.java | 12 ------------ 4 files changed, 3 insertions(+), 36 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java index 08a80a52d5e3..ce06911f67a5 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java +++ b/api/src/main/java/org/apache/cloudstack/acl/APIChecker.java @@ -45,6 +45,6 @@ public interface APIChecker extends Adapter { List getApisAllowedToUser(Role role, User user, List apiNames) throws PermissionDeniedException; boolean isEnabled(); - Pair> getRolePermissions(long roleId); - boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions); + default Pair> getRolePermissions(long roleId) { return null; } + default boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { return false; } } diff --git a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java index 3b98e736ec41..2e7ae23d6f1b 100644 --- a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java +++ b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java @@ -21,8 +21,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; - import org.apache.cloudstack.acl.RolePermissionEntity.Permission; + import org.apache.cloudstack.context.CallContext; import com.cloud.exception.PermissionDeniedException; @@ -33,7 +33,6 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; -import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.PluggableService; @@ -196,14 +195,4 @@ public List getServices() { public void setServices(List services) { this.services = services; } - - @Override - public Pair> getRolePermissions(long roleId) { - return null; - } - - @Override - public boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { - return false; - } } diff --git a/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java b/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java index cb335a948e2c..3444f967d784 100644 --- a/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java +++ b/plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java @@ -33,7 +33,6 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; -import com.cloud.utils.Pair; import com.cloud.utils.PropertiesUtil; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.PluggableService; @@ -201,13 +200,4 @@ public void setCommandPropertyFiles(Set commandPropertyFiles) { this.commandPropertyFiles = commandPropertyFiles; } - @Override - public Pair> getRolePermissions(long roleId) { - return null; - } - - @Override - public boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { - return false; - } } diff --git a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java index 901159a1de73..917cd7bb2b46 100644 --- a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java +++ b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java @@ -27,7 +27,6 @@ import net.sf.ehcache.CacheManager; import org.apache.cloudstack.acl.Role; -import org.apache.cloudstack.acl.RolePermission; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.springframework.stereotype.Component; @@ -43,7 +42,6 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; -import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; @Component @@ -258,14 +256,4 @@ public void setEnabled(boolean enabled) { this.enabled = enabled; } - - @Override - public Pair> getRolePermissions(long roleId) { - return null; - } - - @Override - public boolean checkAccess(Account account, String commandName, Role accountRole, List allPermissions) { - return false; - } } From d395684448c95f5e5b3d5f60f74b922d50753299 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 20 Feb 2026 09:54:55 +0100 Subject: [PATCH 3/4] Update server/src/main/java/com/cloud/user/AccountManagerImpl.java --- server/src/main/java/com/cloud/user/AccountManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index b4b8be61b157..b9a46b8a3730 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1447,7 +1447,7 @@ private void checkApiAccess(APIChecker apiChecker, Account caller, Account reque for (String command : apiNameList) { try { if (roleAndPermissionsForRequested == null) { - apiChecker.checkAccess(caller, command); + apiChecker.checkAccess(requested, command); } else { apiChecker.checkAccess(caller, command, roleAndPermissionsForRequested.first(), roleAndPermissionsForRequested.second()); } From 37fe152c5b8340b8359832e0ae0fe716c1ea388a Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 20 Feb 2026 09:55:02 +0100 Subject: [PATCH 4/4] Update server/src/main/java/com/cloud/user/AccountManagerImpl.java --- server/src/main/java/com/cloud/user/AccountManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index b9a46b8a3730..22cdde26c667 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1449,7 +1449,7 @@ private void checkApiAccess(APIChecker apiChecker, Account caller, Account reque if (roleAndPermissionsForRequested == null) { apiChecker.checkAccess(requested, command); } else { - apiChecker.checkAccess(caller, command, roleAndPermissionsForRequested.first(), roleAndPermissionsForRequested.second()); + apiChecker.checkAccess(requested, command, roleAndPermissionsForRequested.first(), roleAndPermissionsForRequested.second()); } } catch (PermissionDeniedException pde) { continue;