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()));