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 660f64f43ef..ce06911f67a 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 @@ 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 getApisAllowedToUser(Role role, User user, List apiNames) throws PermissionDeniedException; boolean isEnabled(); + + 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/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 030e0bcf014..8ae1ee19b24 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 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API 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 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 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 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/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index d4c23e8d62b..01f83d047c1 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.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 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()));