diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index a3d31a6c093..6f3a0e27fa1 100755 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -39,6 +39,7 @@ import javax.naming.ConfigurationException; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.SecurityChecker; +import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupService; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; @@ -4326,7 +4327,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new InvalidParameterValueException("Can't update system networks"); } - _accountMgr.checkAccess(caller, null, network); + _accountMgr.checkAccess(caller, AccessType.ListEntry, network); List offeringIds = _networkModel.listNetworkOfferingsForUpgrade(networkId); diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 3ab26ccd52a..b5fdc3a9163 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -46,6 +46,7 @@ import org.apache.cloudstack.acl.QuerySelector; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.acl.SecurityChecker.AccessType; +import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd; @@ -89,6 +90,7 @@ import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IpAddress; import com.cloud.network.IpAddressManager; +import com.cloud.network.Network; import com.cloud.network.VpnUserVO; import com.cloud.network.as.AutoScaleManager; import com.cloud.network.dao.AccountGuestVlanMapDao; @@ -123,6 +125,7 @@ import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.snapshot.SnapshotManager; import com.cloud.template.TemplateManager; +import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account.State; import com.cloud.user.dao.AccountDao; import com.cloud.user.dao.UserAccountDao; @@ -484,13 +487,67 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } entityBuf.append("}"); String entityStr = entityBuf.toString(); - for (SecurityChecker checker : _securityCheckers) { - if (checker.checkAccess(caller, accessType, apiName, entities)) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Access to " + entityStr + " granted to " + caller + " by " + checker.getName()); + + boolean isRootAdmin = isRootAdmin(caller.getAccountId()); + boolean isDomainAdmin = isDomainAdmin(caller.getAccountId()); + boolean isResourceDomainAdmin = isResourceDomainAdmin(caller.getAccountId()); + + if ((isRootAdmin || isDomainAdmin || isResourceDomainAdmin || caller.getId() == Account.ACCOUNT_ID_SYSTEM) + && (accessType == null || accessType == AccessType.UseEntry)) { + + for (ControlledEntity entity : entities) { + if (entity instanceof VirtualMachineTemplate || entity instanceof Network + || entity instanceof AffinityGroup) { + for (SecurityChecker checker : _securityCheckers) { + if (checker.checkAccess(caller, accessType, apiName, entity)) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Access to " + entityStr + " granted to " + caller + " by " + + checker.getName()); + } + granted = true; + break; + } + } + } else { + if (isRootAdmin || caller.getId() == Account.ACCOUNT_ID_SYSTEM) { + // no need to make permission checks if the system/root + // admin makes the call + if (s_logger.isTraceEnabled()) { + s_logger.trace("No need to make permission check for System/RootAdmin account, returning true"); + } + granted = true; + } else if (isDomainAdmin || isResourceDomainAdmin) { + Domain entityDomain = getEntityDomain(entity); + if (entityDomain != null) { + try { + checkAccess(caller, entityDomain); + granted = true; + } catch (PermissionDeniedException e) { + List entityList = new ArrayList(); + entityList.add(entity); + e.addDetails(caller, entityList); + throw e; + } + } + } + } + + if (!granted) { + assert false : "How can all of the security checkers pass on checking this check: " + entityStr; + throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to " + + entityStr); + } + + } + } else { + for (SecurityChecker checker : _securityCheckers) { + if (checker.checkAccess(caller, accessType, apiName, entities)) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Access to " + entityStr + " granted to " + caller + " by " + checker.getName()); + } + granted = true; + break; } - granted = true; - break; } } @@ -500,6 +557,27 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } } + private Domain getEntityDomain(ControlledEntity entity) { + Domain entityDomain = null; + long domainId = entity.getDomainId(); + + if (domainId != -1) { + entityDomain = _domainMgr.getDomain(domainId); + } else { + if (entity.getAccountId() != -1) { + // If account exists domainId should too so + // calculate + // it. This condition might be hit for templates or + // entities which miss domainId in their tables + Account account = getAccount(entity.getAccountId()); + domainId = account != null ? account.getDomainId() : -1; + entityDomain = _domainMgr.getDomain(domainId); + } + } + + return entityDomain; + } + @Override public Long checkAccessAndSpecifyAuthority(Account caller, Long zoneId) { // We just care for resource domain admin for now. He should be permitted to see only his zone. diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java index 3a3ba4dd3f2..91593f6f23e 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java +++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java @@ -247,12 +247,8 @@ public class RoleBasedAPIAccessChecker extends AdapterBase implements APIChecker try { cmdObj = (BaseCmd) cmdClass.newInstance(); if (cmdObj instanceof BaseListCmd) { - if (permissionScope == PermissionScope.ACCOUNT) { - accessType = AccessType.UseEntry; - } else { - accessType = AccessType.ListEntry; - addAccountScopedUseEntry = true; - } + accessType = AccessType.ListEntry; + addAccountScopedUseEntry = true; } else { accessType = AccessType.OperateEntry; } diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java index cc29ab541bb..63aa82786f0 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java +++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java @@ -107,22 +107,14 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur permissions = _iamSrv.listPolicyPermissionByActionAndEntity(policy.getId(), action, entityType); if (permissions.isEmpty()) { if (accessType != null) { - for (AccessType type : AccessType.values()) { - if (type.ordinal() >= accessType.ordinal()) { - permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), - type.toString(), entityType)); - } - } + permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), + accessType.toString(), entityType)); } } } else { if (accessType != null) { - for (AccessType type : AccessType.values()) { - if (type.ordinal() >= accessType.ordinal()) { - permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), - type.toString(), entityType)); - } - } + permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), + accessType.toString(), entityType)); } } for (IAMPolicyPermission permission : permissions) { diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityQuerySelector.java b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityQuerySelector.java index b7c3d352773..5d31433803f 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityQuerySelector.java +++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityQuerySelector.java @@ -57,11 +57,9 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe List domainIds = new ArrayList(); for (IAMPolicy policy : policies) { List pp = new ArrayList(); - for (AccessType type : AccessType.values()) { - if (type.ordinal() >= accessType.ordinal()) { - pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.DOMAIN.toString(), type)); - } - } + pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, + PermissionScope.DOMAIN.toString(), accessType.toString())); + if (pp != null) { for (IAMPolicyPermission p : pp) { if (p.getScopeId() != null) { @@ -101,11 +99,9 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe List accountIds = new ArrayList(); for (IAMPolicy policy : policies) { List pp = new ArrayList(); - for (AccessType type : AccessType.values()) { - if (type.ordinal() >= accessType.ordinal()) { - pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ACCOUNT.toString(), type)); - } - } + pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, + PermissionScope.ACCOUNT.toString(), accessType.toString())); + if (pp != null) { for (IAMPolicyPermission p : pp) { if (p.getScopeId() != null) { @@ -144,11 +140,9 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe List entityIds = new ArrayList(); for (IAMPolicy policy : policies) { List pp = new ArrayList(); - for (AccessType type : AccessType.values()) { - if (type.ordinal() >= accessType.ordinal()) { - pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.RESOURCE.toString(), type)); - } - } + pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, + PermissionScope.RESOURCE.toString(), accessType.toString())); + if (pp != null) { for (IAMPolicyPermission p : pp) { if (p.getScopeId() != null) { @@ -171,13 +165,10 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe // for each policy, find granted permission with ALL scope for (IAMPolicy policy : policies) { List pp = new ArrayList(); - for (AccessType type : AccessType.values()) { - if (type.ordinal() >= accessType.ordinal()) { - pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ALL.toString(), type)); - } - if (pp != null && pp.size() > 0) { - return true; - } + pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ALL.toString(), + accessType.toString())); + if (pp != null && pp.size() > 0) { + return true; } } return false; diff --git a/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java b/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java index c396fa922fe..20326e974c0 100644 --- a/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java +++ b/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java @@ -18,7 +18,7 @@ package org.apache.cloudstack.iam.api; import java.util.List; -import org.apache.cloudstack.acl.SecurityChecker.AccessType; + import org.apache.cloudstack.iam.api.IAMPolicyPermission.Permission; import com.cloud.utils.Pair; @@ -73,7 +73,7 @@ public interface IAMService { List listPolicyPermissions(long policyId); - List listPolicyPermissionsByScope(long policyId, String action, String scope, AccessType accessType); + List listPolicyPermissionsByScope(long policyId, String action, String scope, String accessType); List listPolicyPermissionByActionAndEntity(long policyId, String action, String entityType); diff --git a/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java b/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java index d6a61a10457..c35ac1d2734 100644 --- a/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java +++ b/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java @@ -25,7 +25,6 @@ import javax.inject.Inject; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.PermissionScope; -import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.iam.api.IAMGroup; import org.apache.cloudstack.iam.api.IAMPolicy; import org.apache.cloudstack.iam.api.IAMPolicyPermission; @@ -632,7 +631,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } // add entry in acl_policy_permission table - IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope, scopeId, action, perm); + IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope, + scopeId, action, perm, accessType); if (permit == null) { // not there already permit = new IAMPolicyPermissionVO(iamPolicyId, action, entityType, accessType, scope, scopeId, perm, @@ -654,7 +654,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { + "; failed to revoke permission from policy."); } // remove entry from acl_entity_permission table - IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope, scopeId, action, Permission.Allow); + IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope, + scopeId, action, Permission.Allow, null); if (permit != null) { // not removed yet _policyPermissionDao.remove(permit.getId()); @@ -766,9 +767,10 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { @SuppressWarnings("unchecked") @Override - public List listPolicyPermissionsByScope(long policyId, String action, String scope, AccessType accessType) { + public List listPolicyPermissionsByScope(long policyId, String action, String scope, + String accessType) { @SuppressWarnings("rawtypes") - List pp = _policyPermissionDao.listByPolicyActionAndScope(policyId, action, scope, accessType.toString()); + List pp = _policyPermissionDao.listByPolicyActionAndScope(policyId, action, scope, accessType); return pp; } diff --git a/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDao.java b/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDao.java index 6e6099c6648..ebb4916470d 100644 --- a/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDao.java +++ b/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDao.java @@ -27,7 +27,7 @@ public interface IAMPolicyPermissionDao extends GenericDao listByPolicy(long policyId); IAMPolicyPermissionVO findByPolicyAndEntity(long policyId, String entityType, String scope, Long scopeId, - String action, Permission perm); + String action, Permission perm, String accessType); List listByPolicyActionAndScope(long policyId, String action, String scope, String accessType); diff --git a/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDaoImpl.java b/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDaoImpl.java index 9cc1af07e8c..44b77d1163e 100644 --- a/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDaoImpl.java +++ b/services/iam/server/src/org/apache/cloudstack/iam/server/dao/IAMPolicyPermissionDaoImpl.java @@ -70,7 +70,7 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase sc = fullSearch.create(); sc.setParameters("policyId", policyId); sc.setParameters("entityType", entityType); @@ -78,6 +78,11 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase