diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index cda82f52a2a..daa843f8954 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -97,7 +97,7 @@ public interface AccountService { void checkAccess(Account account, AccessType accessType, boolean sameOwner, ControlledEntity... entities) throws PermissionDeniedException; - void checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException; + void checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException; void checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException; diff --git a/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java b/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java index e4f3ca8075e..b1f95b3399f 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java +++ b/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java @@ -136,7 +136,7 @@ public interface SecurityChecker extends Adapter { boolean checkAccess(Account account, DataCenter zone) throws PermissionDeniedException; - public boolean checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException; + boolean checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException; boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException; } diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index c75d52c06d8..a4825bd6be2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -113,7 +113,6 @@ public class ApiConstants { public static final String DOMAIN_PATH = "domainpath"; public static final String DOMAIN_ID = "domainid"; public static final String DOMAIN__ID = "domainId"; - public static final String DOMAIN_ID_LIST = "domainids"; public static final String DURATION = "duration"; public static final String ELIGIBLE = "eligible"; public static final String EMAIL = "email"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java index 508ce821f15..ec94355d419 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java @@ -29,6 +29,7 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.ServiceOfferingResponse; +import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; @@ -90,6 +91,14 @@ public class CreateServiceOfferingCmd extends BaseCmd { description = "the ID of the containing domain(s), null for public offerings") private List domainIds; + @Parameter(name = ApiConstants.ZONE_ID, + type = CommandType.LIST, + collectionType = CommandType.UUID, + entityType = ZoneResponse.class, + description = "the ID of the containing zone(s), null for public offerings", + since = "4.13") + private List zoneIds; + @Parameter(name = ApiConstants.HOST_TAGS, type = CommandType.STRING, description = "the host tag for this service offering.") private String hostTag; @@ -251,10 +260,14 @@ public class CreateServiceOfferingCmd extends BaseCmd { return tags; } - public List getDomainId() { + public List getDomainIds() { return domainIds; } + public List getZoneIds() { + return zoneIds; + } + public String getHostTag() { return hostTag; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java index 4b7481974f2..dcb1730cec6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.user.offering; +import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.log4j.Logger; import org.apache.cloudstack.api.APICommand; @@ -57,6 +58,13 @@ public class ListServiceOfferingsCmd extends BaseListDomainResourcesCmd { description = "the system VM type. Possible types are \"consoleproxy\", \"secondarystoragevm\" or \"domainrouter\".") private String systemVmType; + @Parameter(name = ApiConstants.ZONE_ID, + type = CommandType.UUID, + entityType = ZoneResponse.class, + description = "id of zone disk offering is associated with", + since = "4.13") + private Long zoneId; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -81,6 +89,10 @@ public class ListServiceOfferingsCmd extends BaseListDomainResourcesCmd { return systemVmType; } + public Long getZoneId() { + return zoneId; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDetailsDaoImpl.java index 66081e17198..76840267461 100644 --- a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDetailsDaoImpl.java @@ -38,8 +38,8 @@ public class ServiceOfferingDetailsDaoImpl extends ResourceDetailsDaoBase findDomainIds(long resourceId) { final List domainIds = new ArrayList<>(); for (final ServiceOfferingDetailsVO detail: findDetails(resourceId, ApiConstants.DOMAIN_ID)) { - final Long domainId = Long.valueOf(detail.getValue(), -1); - if (domainId != -1) { + final Long domainId = Long.valueOf(detail.getValue()); + if (domainId > 0) { domainIds.add(domainId); } } @@ -50,8 +50,8 @@ public class ServiceOfferingDetailsDaoImpl extends ResourceDetailsDaoBase findZoneIds(long resourceId) { final List zoneIds = new ArrayList<>(); for (final ServiceOfferingDetailsVO detail: findDetails(resourceId, ApiConstants.ZONE_ID)) { - final Long zoneId = Long.valueOf(detail.getValue(), -1); - if (zoneId != -1) { + final Long zoneId = Long.valueOf(detail.getValue()); + if (zoneId > 0) { zoneIds.add(zoneId); } } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java index df6018ccfe1..d93a05200f6 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java @@ -18,12 +18,10 @@ package com.cloud.storage.dao; import java.util.Date; import java.util.List; -import java.util.Map; import javax.inject.Inject; import javax.persistence.EntityExistsException; -import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; @@ -35,7 +33,6 @@ import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.SearchCriteria.Op; -import com.google.common.base.Strings; @Component public class DiskOfferingDaoImpl extends GenericDaoBase implements DiskOfferingDao { @@ -102,16 +99,8 @@ public class DiskOfferingDaoImpl extends GenericDaoBase im SearchCriteria sc = PublicDiskOfferingSearch.create(); sc.setParameters("system", false); List offerings = listBy(sc); - if(offerings!=null) { - for (int i = offerings.size() - 1; i >= 0; i--) { - DiskOfferingVO offering = offerings.get(i); - Map offeringDetails = detailsDao.listDetailsKeyPairs(offering.getId()); - if (!Strings.isNullOrEmpty(offeringDetails.get(ApiConstants.DOMAIN_ID_LIST))) { - // TODO: For ROOT domainId? - offerings.remove(i); - } - } + offerings.removeIf(o -> (!detailsDao.findDomainIds(o.getId()).isEmpty())); } return offerings; } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/dao/DiskOfferingDetailsDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/dao/DiskOfferingDetailsDaoImpl.java index e0fb6592a8d..da0ec5bc580 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/dao/DiskOfferingDetailsDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/dao/DiskOfferingDetailsDaoImpl.java @@ -37,8 +37,8 @@ public class DiskOfferingDetailsDaoImpl extends ResourceDetailsDaoBase findDomainIds(long resourceId) { final List domainIds = new ArrayList<>(); for (final DiskOfferingDetailVO detail: findDetails(resourceId, ApiConstants.DOMAIN_ID)) { - final Long domainId = Long.valueOf(detail.getValue(), -1); - if (domainId != -1) { + final Long domainId = Long.valueOf(detail.getValue()); + if (domainId > 0) { domainIds.add(domainId); } } @@ -49,8 +49,8 @@ public class DiskOfferingDetailsDaoImpl extends ResourceDetailsDaoBase findZoneIds(long resourceId) { final List zoneIds = new ArrayList<>(); for (final DiskOfferingDetailVO detail: findDetails(resourceId, ApiConstants.ZONE_ID)) { - final Long zoneId = Long.valueOf(detail.getValue(), -1); - if (zoneId != -1) { + final Long zoneId = Long.valueOf(detail.getValue()); + if (zoneId > 0) { zoneIds.add(zoneId); } } diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index 7320773b858..afcc5856500 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -431,7 +431,7 @@ public class MockAccountManager extends ManagerBase implements AccountManager { } @Override - public void checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException { + public void checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException { // TODO Auto-generated method stub } diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 74092bdde69..384c2c22d0b 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -16,16 +16,13 @@ // under the License. package com.cloud.acl; -import java.util.Arrays; import java.util.List; -import java.util.Map; import javax.inject.Inject; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroup; -import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; @@ -50,7 +47,6 @@ import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.component.AdapterBase; -import com.google.common.base.Strings; @Component public class DomainChecker extends AdapterBase implements SecurityChecker { @@ -180,8 +176,8 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { @Override public boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException { boolean isAccess = false; - Map details = null; - if (account == null || dof == null) {//public offering + // Check fo domains + if (account == null || dof == null) { isAccess = true; } else { //admin has all permissions @@ -189,78 +185,68 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { isAccess = true; } //if account is normal user or domain admin - //check if account's domain is a child of zone's domain (Note: This is made consistent with the list command for disk offering) + //check if account's domain is a child of offering's domain (Note: This is made consistent with the list command for disk offering) else if (_accountService.isNormalUser(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN || _accountService.isDomainAdmin(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_PROJECT) { - details = diskOfferingDetailsDao.listDetailsKeyPairs(dof.getId()); - isAccess = true; - if (details.containsKey(ApiConstants.DOMAIN_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { - List domainIds = Arrays.asList(details.get(ApiConstants.DOMAIN_ID_LIST).split(",")); - for (String domainId : domainIds) { - if (!_domainDao.isChildDomain(Long.valueOf(domainId), account.getDomainId())) { - isAccess = false; + final List doDomainIds = diskOfferingDetailsDao.findDomainIds(dof.getId()); + if (doDomainIds.isEmpty()) { + isAccess = true; + } else { + for (Long domainId : doDomainIds) { + if (_domainDao.isChildDomain(domainId, account.getDomainId())) { + isAccess = true; break; } } } } } - // Check for zones if (isAccess && dof != null && zone != null) { - if (details == null) - details = diskOfferingDetailsDao.listDetailsKeyPairs(dof.getId()); - if (details.containsKey(ApiConstants.ZONE_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { - List zoneIds = Arrays.asList(details.get(ApiConstants.ZONE_ID_LIST).split(",")); - isAccess = zoneIds.contains(String.valueOf(zone.getId())); - } + final List doZoneIds = diskOfferingDetailsDao.findZoneIds(dof.getId()); + isAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId()); } - //not found return isAccess; } @Override - public boolean checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException { - final List soDomainIds = serviceOfferingDetailsDao.findDomainIds(so.getId()); - if (account == null || soDomainIds.isEmpty()) { //public offering - return true; + public boolean checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException { + boolean isAccess = false; + // Check fo domains + if (account == null || so == null) { + isAccess = true; } else { //admin has all permissions if (_accountService.isRootAdmin(account.getId())) { - return true; + isAccess = true; } //if account is normal user or domain admin - //check if account's domain is a child of zone's domain (Note: This is made consistent with the list command for service offering) + //check if account's domain is a child of offering's domain (Note: This is made consistent with the list command for service offering) else if (_accountService.isNormalUser(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN || _accountService.isDomainAdmin(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_PROJECT) { - if (soDomainIds.contains(account.getDomainId())) { - return true; //service offering and account at exact node + final List soDomainIds = serviceOfferingDetailsDao.findDomainIds(so.getId()); + if (soDomainIds.isEmpty()) { + isAccess = true; } else { - Domain domainRecord = _domainDao.findById(account.getDomainId()); - if (domainRecord != null) { - while (true) { - if (soDomainIds.contains(domainRecord.getId())) { - //found as a child - return true; - } - if (domainRecord.getParent() != null) { - domainRecord = _domainDao.findById(domainRecord.getParent()); - } else { - break; - } + for (Long domainId : soDomainIds) { + if (_domainDao.isChildDomain(domainId, account.getDomainId())) { + isAccess = true; + break; } } } } } - //not found - return false; + // Check for zones + if (isAccess && so != null && zone != null) { + final List soZoneIds = serviceOfferingDetailsDao.findZoneIds(so.getId()); + isAccess = soZoneIds.isEmpty() || soZoneIds.contains(zone.getId()); + } + return isAccess; } @Override diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 2fb337025d6..a97d8aec74a 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.ListIterator; import java.util.Map; import java.util.Set; @@ -31,7 +32,6 @@ import org.apache.cloudstack.affinity.AffinityGroupResponse; import org.apache.cloudstack.affinity.AffinityGroupVMMapVO; import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; -import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd; import org.apache.cloudstack.api.ResourceDetail; import org.apache.cloudstack.api.ResponseObject.ResponseView; @@ -108,6 +108,7 @@ import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; +import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -2527,6 +2528,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q Long domainId = cmd.getDomainId(); Boolean isRootAdmin = _accountMgr.isRootAdmin(account.getAccountId()); Boolean isRecursive = cmd.isRecursive(); + Long zoneId = cmd.getZoneId(); // Keeping this logic consistent with domain specific zones // if a domainId is provided, we just return the disk offering // associated with this domain @@ -2534,7 +2536,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q if (_accountMgr.isRootAdmin(account.getId()) || isPermissible(account.getDomainId(), domainId)) { // check if the user's domain == do's domain || user's domain is // a child of so's domain for non-root users - sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId); + sc.addAnd("domainId", Op.FIND_IN_SET, String.valueOf(domainId)); if (!isRootAdmin) { sc.addAnd("displayOffering", SearchCriteria.Op.EQ, 1); } @@ -2573,6 +2575,16 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q sc.addAnd("name", SearchCriteria.Op.EQ, name); } + if (zoneId != null) { + SearchBuilder sb = _diskOfferingJoinDao.createSearchBuilder(); + sb.and("zoneId", sb.entity().getZoneId(), Op.FIND_IN_SET); + sb.or("zId", sb.entity().getZoneId(), Op.NULL); + sb.done(); + SearchCriteria zoneSC = sb.create(); + zoneSC.setParameters("zoneId", String.valueOf(zoneId)); + sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC); + } + // FIXME: disk offerings should search back up the hierarchy for // available disk offerings... /* @@ -2598,20 +2610,16 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q Pair, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter); // Remove offerings that are not associated with caller's domain and passed zone // TODO: Better approach - if (result.first() != null && !result.first().isEmpty()) { - List offerings = result.first(); - for (int i = offerings.size() - 1; i >= 0; i--) { - DiskOfferingJoinVO offering = offerings.get(i); - Map details = diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); - boolean toRemove = account.getType() == Account.ACCOUNT_TYPE_ADMIN ? false : isRecursive; - if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && - details.containsKey(ApiConstants.DOMAIN_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { - toRemove = true; - String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); - for (String dIdStr : domainIdsArray) { - Long dId = Long.valueOf(dIdStr.trim()); - if(isRecursive) { + if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(result.first())) { + ListIterator it = result.first().listIterator(); + while (it.hasNext()) { + DiskOfferingJoinVO offering = it.next(); + if(!Strings.isNullOrEmpty(offering.getDomainId())) { + boolean toRemove = true; + String[] domainIdsArray = offering.getDomainId().split(","); + for (String domainIdString : domainIdsArray) { + Long dId = Long.valueOf(domainIdString.trim()); + if (isRecursive) { if (_domainDao.isChildDomain(account.getDomainId(), dId)) { toRemove = false; break; @@ -2623,28 +2631,13 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q } } } - } - if (toRemove) { - offerings.remove(i); - } else { - // If zoneid is passed remove offerings that are not associated with the zone - if (cmd.getZoneId() != null) { - final Long zoneId = cmd.getZoneId(); - if (details.containsKey(ApiConstants.ZONE_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { - String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); - List zoneIds = new ArrayList<>(); - for (String zId : zoneIdsArray) - zoneIds.add(Long.valueOf(zId.trim())); - if (!zoneIds.contains(zoneId)) { - offerings.remove(i); - } - } + if (toRemove) { + it.remove(); } } } } - return result; + return new Pair<>(result.first(), result.first().size()); } private List filterOfferingsOnCurrentTags(List offerings, ServiceOfferingVO currentVmOffering) { @@ -2698,6 +2691,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q String vmTypeStr = cmd.getSystemVmType(); ServiceOfferingVO currentVmOffering = null; Boolean isRecursive = cmd.isRecursive(); + Long zoneId = cmd.getZoneId(); SearchCriteria sc = _srvOfferingJoinDao.createSearchCriteria(); if (!_accountMgr.isRootAdmin(caller.getId()) && isSystem) { @@ -2749,9 +2743,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL) { throw new InvalidParameterValueException("Only ROOT admins and Domain admins can list service offerings with isrecursive=true"); } - DomainVO domainRecord = _domainDao.findById(caller.getDomainId()); - sc.addAnd("domainPath", SearchCriteria.Op.LIKE, domainRecord.getPath() + "%"); - } else { // domain + all ancestors + }/* else { // domain + all ancestors // find all domain Id up to root domain for this account List domainIds = new ArrayList(); DomainVO domainRecord; @@ -2779,14 +2771,14 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q spc.addOr("domainId", SearchCriteria.Op.IN, domainIds.toArray()); spc.addOr("domainId", SearchCriteria.Op.NULL); // include public offering as well sc.addAnd("domainId", SearchCriteria.Op.SC, spc); - } + }*/ } else { // for root users if (caller.getDomainId() != 1 && isSystem) { // NON ROOT admin throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering"); } if (domainId != null) { - sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId); + sc.addAnd("domainId", Op.FIND_IN_SET, String.valueOf(domainId)); } } @@ -2816,11 +2808,50 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q sc.addAnd("vmType", SearchCriteria.Op.EQ, vmTypeStr); } + if (zoneId != null) { + SearchBuilder sb = _srvOfferingJoinDao.createSearchBuilder(); + sb.and("zoneId", sb.entity().getZoneId(), Op.FIND_IN_SET); + sb.or("zId", sb.entity().getZoneId(), Op.NULL); + sb.done(); + SearchCriteria zoneSC = sb.create(); + zoneSC.setParameters("zoneId", String.valueOf(zoneId)); + sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC); + } + Pair, Integer> result = _srvOfferingJoinDao.searchAndCount(sc, searchFilter); //Couldn't figure out a smart way to filter offerings based on tags in sql so doing it in Java. List filteredOfferings = filterOfferingsOnCurrentTags(result.first(), currentVmOffering); - return new Pair<>(filteredOfferings, result.second()); + // Remove offerings that are not associated with caller's domain + // TODO: Better approach + if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(filteredOfferings)) { + ListIterator it = filteredOfferings.listIterator(); + while (it.hasNext()) { + ServiceOfferingJoinVO offering = it.next(); + if(!Strings.isNullOrEmpty(offering.getDomainId())) { + boolean toRemove = true; + String[] domainIdsArray = offering.getDomainId().split(","); + for (String domainIdString : domainIdsArray) { + Long dId = Long.valueOf(domainIdString.trim()); + if (isRecursive) { + if (_domainDao.isChildDomain(caller.getDomainId(), dId)) { + toRemove = false; + break; + } + } else { + if (_domainDao.isChildDomain(dId, caller.getDomainId())) { + toRemove = false; + break; + } + } + } + if (toRemove) { + it.remove(); + } + } + } + } + return new Pair<>(filteredOfferings, filteredOfferings.size()); } @Override diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 7738791e2e4..617f96bed76 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -86,6 +86,7 @@ import org.apache.cloudstack.region.PortableIpVO; import org.apache.cloudstack.region.Region; import org.apache.cloudstack.region.RegionVO; import org.apache.cloudstack.region.dao.RegionDao; +import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; @@ -2289,14 +2290,22 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } // check if valid domain - if (cmd.getDomainId() != null && !cmd.getDomainId().isEmpty()) { - for (final Long domainId: cmd.getDomainId()) { + if (CollectionUtils.isNotEmpty(cmd.getDomainIds())) { + for (final Long domainId: cmd.getDomainIds()) { if (_domainDao.findById(domainId) == null) { throw new InvalidParameterValueException("Please specify a valid domain id"); } } } + // check if valid zone + if (CollectionUtils.isNotEmpty(cmd.getZoneIds())) { + for (Long zoneId : cmd.getZoneIds()) { + if (_zoneDao.findById(zoneId) == null) + throw new InvalidParameterValueException("Please specify a valid zone id"); + } + } + final Boolean offerHA = cmd.isOfferHa(); boolean localStorageRequired = false; @@ -2371,7 +2380,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } return createServiceOffering(userId, cmd.isSystem(), vmType, cmd.getServiceOfferingName(), cpuNumber, memory, cpuSpeed, cmd.getDisplayText(), - cmd.getProvisioningType(), localStorageRequired, offerHA, limitCpuUse, volatileVm, cmd.getTags(), cmd.getDomainId(), cmd.getHostTag(), + cmd.getProvisioningType(), localStorageRequired, offerHA, limitCpuUse, volatileVm, cmd.getTags(), cmd.getDomainIds(), cmd.getZoneIds(), cmd.getHostTag(), cmd.getNetworkRate(), cmd.getDeploymentPlanner(), details, isCustomizedIops, cmd.getMinIops(), cmd.getMaxIops(), cmd.getBytesReadRate(), cmd.getBytesReadRateMax(), cmd.getBytesReadRateMaxLength(), cmd.getBytesWriteRate(), cmd.getBytesWriteRateMax(), cmd.getBytesWriteRateMaxLength(), @@ -2382,13 +2391,15 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati protected ServiceOfferingVO createServiceOffering(final long userId, final boolean isSystem, final VirtualMachine.Type vmType, final String name, final Integer cpu, final Integer ramSize, final Integer speed, final String displayText, final String provisioningType, final boolean localStorageRequired, - final boolean offerHA, final boolean limitResourceUse, final boolean volatileVm, String tags, final List domainIds, final String hostTag, + final boolean offerHA, final boolean limitResourceUse, final boolean volatileVm, String tags, final List domainIds, List zoneIds, final String hostTag, final Integer networkRate, final String deploymentPlanner, final Map details, final Boolean isCustomizedIops, Long minIops, Long maxIops, Long bytesReadRate, Long bytesReadRateMax, Long bytesReadRateMaxLength, Long bytesWriteRate, Long bytesWriteRateMax, Long bytesWriteRateMaxLength, Long iopsReadRate, Long iopsReadRateMax, Long iopsReadRateMaxLength, Long iopsWriteRate, Long iopsWriteRateMax, Long iopsWriteRateMaxLength, final Integer hypervisorSnapshotReserve) { + // Filter child domains when both parent and child domains are present + List filteredDomainIds = filterChildSubDomains(domainIds); // Check if user exists in the system final User user = _userDao.findById(userId); @@ -2397,14 +2408,14 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } final Account account = _accountDao.findById(user.getAccountId()); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (domainIds == null || domainIds.isEmpty()) { + if (filteredDomainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to create public service offering by id " + userId + " because it is domain-admin"); } if (tags != null || hostTag != null) { throw new InvalidParameterValueException("Unable to create service offering with storage tags or host tags by id " + userId + " because it is domain-admin"); } - for (Long domainId: domainIds) { - if (! _domainDao.isChildDomain(account.getDomainId(), domainId)) { + for (Long domainId : filteredDomainIds) { + if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { throw new InvalidParameterValueException("Unable to create service offering by another domain admin with id " + userId); } } @@ -2515,13 +2526,16 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } if ((offering = _serviceOfferingDao.persist(offering)) != null) { - if (domainIds != null && !domainIds.isEmpty()) { - for (Long domainId: domainIds) { - detailsVO.add(new ServiceOfferingDetailsVO(offering.getId(), ApiConstants.DOMAIN_ID, String.valueOf(domainId), true)); + for (Long domainId : filteredDomainIds) { + detailsVO.add(new ServiceOfferingDetailsVO(offering.getId(), ApiConstants.DOMAIN_ID, String.valueOf(domainId), true)); + } + if (CollectionUtils.isNotEmpty(zoneIds)) { + for (Long zoneId : zoneIds) { + detailsVO.add(new ServiceOfferingDetailsVO(offering.getId(), ApiConstants.ZONE_ID, String.valueOf(zoneId), true)); } } if (!detailsVO.isEmpty()) { - for (ServiceOfferingDetailsVO detail: detailsVO) { + for (ServiceOfferingDetailsVO detail : detailsVO) { detail.setResourceId(offering.getId()); } _serviceOfferingDetailsDao.saveDetails(detailsVO); @@ -2696,13 +2710,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new InvalidParameterValueException("Unable to create disk offering by id " + userId + " because it is not root-admin or domain-admin"); } - if (zoneIds != null) { - for (Long zoneId : zoneIds) { - if (_zoneDao.findById(zoneId) == null) - throw new InvalidParameterValueException("Unable to create disk offering associated with invalid zone, " + zoneId); - } - } - tags = StringUtils.cleanupTags(tags); final DiskOfferingVO newDiskOffering = new DiskOfferingVO(name, description, typedProvisioningType, diskSize, tags, isCustomized, isCustomizedIops, minIops, maxIops); @@ -2755,17 +2762,17 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati CallContext.current().setEventDetails("Disk offering id=" + newDiskOffering.getId()); final DiskOfferingVO offering = _diskOfferingDao.persist(newDiskOffering); if (offering != null) { - if(!filteredDomainIds.isEmpty()) { - List domainIdsStringList = new ArrayList<>(); - for(Long domainId : filteredDomainIds) - domainIdsStringList.add(String.valueOf(domainId)); - diskOfferingDetailsDao.addDetail(offering.getId(), ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsStringList), true); + List detailsVO = new ArrayList<>(); + for (Long domainId : filteredDomainIds) { + detailsVO.add(new DiskOfferingDetailVO(offering.getId(), ApiConstants.DOMAIN_ID, String.valueOf(domainId), true)); } - if(zoneIds!=null && !zoneIds.isEmpty()) { - List zoneIdsStringList = new ArrayList<>(); - for(Long zoneId : zoneIds) - zoneIdsStringList.add(String.valueOf(zoneId)); - diskOfferingDetailsDao.addDetail(offering.getId(), ApiConstants.ZONE_ID_LIST, String.join(",", zoneIdsStringList), true); + if (CollectionUtils.isNotEmpty(zoneIds)) { + for (Long zoneId : zoneIds) { + detailsVO.add(new DiskOfferingDetailVO(offering.getId(), ApiConstants.ZONE_ID, String.valueOf(zoneId), true)); + } + } + if (!detailsVO.isEmpty()) { + diskOfferingDetailsDao.saveDetails(detailsVO); } CallContext.current().setEventDetails("Disk offering id=" + newDiskOffering.getId()); return offering; @@ -2786,6 +2793,23 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati final List domainIds = cmd.getDomainIds(); final List zoneIds = cmd.getZoneIds(); + // check if valid domain + if (CollectionUtils.isNotEmpty(domainIds)) { + for (final Long domainId: domainIds) { + if (_domainDao.findById(domainId) == null) { + throw new InvalidParameterValueException("Please specify a valid domain id"); + } + } + } + + // check if valid zone + if (CollectionUtils.isNotEmpty(zoneIds)) { + for (Long zoneId : zoneIds) { + if (_zoneDao.findById(zoneId) == null) + throw new InvalidParameterValueException("Please specify a valid zone id"); + } + } + if (!isCustomized && numGibibytes == null) { throw new InvalidParameterValueException("Disksize is required for a non-customized disk offering"); } @@ -2840,15 +2864,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati // Check if diskOffering exists final DiskOffering diskOfferingHandle = _entityMgr.findById(DiskOffering.class, diskOfferingId); - List existingDomainIds = new ArrayList<>(); - Map details = diskOfferingDetailsDao.listDetailsKeyPairs(diskOfferingId); - if (details.containsKey(ApiConstants.DOMAIN_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { - String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); - for (String dIdStr : domainIdsArray) { - existingDomainIds.add(Long.valueOf(dIdStr.trim())); - } - } + List existingDomainIds = diskOfferingDetailsDao.findDomainIds(diskOfferingId); if (diskOfferingHandle == null) { throw new InvalidParameterValueException("Unable to find disk offering by id " + diskOfferingId); @@ -2954,15 +2970,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } final Account account = _accountDao.findById(user.getAccountId()); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - List existingDomainIds = new ArrayList<>(); - Map details = diskOfferingDetailsDao.listDetailsKeyPairs(diskOfferingId); - if (details.containsKey(ApiConstants.DOMAIN_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { - String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); - for (String dIdStr : domainIdsArray) { - existingDomainIds.add(Long.valueOf(dIdStr.trim())); - } - } + List existingDomainIds = diskOfferingDetailsDao.findDomainIds(diskOfferingId); if (existingDomainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to delete public disk offering by id " + userId + " because it is domain-admin"); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 5783e4b65f9..8aeb3c682a9 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2830,9 +2830,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } @Override - public void checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException { + public void checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException { for (SecurityChecker checker : _securityCheckers) { - if (checker.checkAccess(account, so)) { + if (checker.checkAccess(account, so, zone)) { if (s_logger.isDebugEnabled()) { s_logger.debug("Access granted to " + account + " to " + so + " by " + checker.getName()); } @@ -2852,8 +2852,6 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M s_logger.debug("Access granted to " + account + " to " + dof + " by " + checker.getName()); } return; - } else { - throw new PermissionDeniedException(String.format("Access denied to %s for disk offering: %s, zone: %s by %s", account, dof.getUuid(), zone.getUuid(), checker.getName())); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 47c370deeb1..7ddc345ac3e 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1110,7 +1110,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Check if the new service offering can be applied to vm instance ServiceOffering newSvcOffering = _offeringDao.findById(svcOffId); Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId()); - _accountMgr.checkAccess(owner, newSvcOffering); + _accountMgr.checkAccess(owner, newSvcOffering, _dcDao.findById(vmInstance.getDataCenterId())); _itMgr.upgradeVmDb(vmId, svcOffId); if (newServiceOffering.isDynamic()) { @@ -3014,7 +3014,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _accountMgr.checkAccess(caller, null, true, owner); // Verify that owner can use the service offering - _accountMgr.checkAccess(owner, serviceOffering); + _accountMgr.checkAccess(owner, serviceOffering, zone); _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId), zone); // Get default guest network in Basic zone @@ -3073,7 +3073,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _accountMgr.checkAccess(caller, null, true, owner); // Verify that owner can use the service offering - _accountMgr.checkAccess(owner, serviceOffering); + _accountMgr.checkAccess(owner, serviceOffering, zone); _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId), zone); // If no network is specified, find system security group enabled network @@ -3181,7 +3181,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _accountMgr.checkAccess(caller, null, true, owner); // Verify that owner can use the service offering - _accountMgr.checkAccess(owner, serviceOffering); + _accountMgr.checkAccess(owner, serviceOffering, zone); _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId), zone); List vpcSupportedHTypes = _vpcMgr.getSupportedVpcHypervisors(); diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index c6088521360..c35fb215e10 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -213,7 +213,7 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco } @Override - public void checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException { + public void checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException { // TODO Auto-generated method stub } diff --git a/ui/scripts/configuration.js b/ui/scripts/configuration.js index cbaebb72a65..414df7ced18 100644 --- a/ui/scripts/configuration.js +++ b/ui/scripts/configuration.js @@ -646,14 +646,18 @@ }); } }, - domainId: { label: 'label.domain', - docID: 'helpComputeOfferingDomain', + docID: 'helpDiskOfferingDomain', dependsOn: 'isPublic', + isMultiple: true, select: function(args) { $.ajax({ - url: createURL("listDomains&listAll=true"), + url: createURL('listDomains'), + data: { + listAll: true, + details: 'min' + }, dataType: "json", async: false, success: function(json) { @@ -675,6 +679,42 @@ }); }, isHidden: true + }, + zoneId: { + label: 'label.zone', + docID: 'helpDiskOfferingZone', + isMultiple: true, + validation: { + allzonesonly: true + }, + select: function(args) { + $.ajax({ + url: createURL("listZones"), + data: {available: 'true'}, + dataType: "json", + async: true, + success: function(json) { + var items = []; + var zoneObjs = json.listzonesresponse.zone; + $(zoneObjs).each(function() { + items.push({ + id: this.id, + description: this.name + }); + }); + items.sort(function(a, b) { + return a.description.localeCompare(b.description); + }); + items.unshift({ + id: -1, + description: "All Zones" + }); + args.response.success({ + data: items + }); + } + }); + } } } }, @@ -832,6 +872,22 @@ }); } + if (args.data.isPublic != "on") { + var domainId = (args.data.domainId && Array.isArray(args.data.domainId)) ? args.data.domainId.join(',') : args.data.domainId; + if (domainId) { + $.extend(data, { + domainid: domainId + }); + } + } + + var zoneId = (args.data.zone && Array.isArray(args.data.zoneId)) ? args.data.zoneId.join(',') : args.data.zoneId; + if (zoneId) { + $.extend(data, { + zoneid: zoneId + }); + } + $.ajax({ url: createURL('createServiceOffering' + array1.join("")), data: data, @@ -1066,6 +1122,9 @@ domain: { label: 'label.domain' }, + zone: { + label: 'label.zone' + }, created: { label: 'label.created', converter: cloudStack.converters.toLocalDate @@ -2106,7 +2165,7 @@ isChecked: false, docID: 'helpDiskOfferingPublic' }, - domain: { + domainId: { label: 'label.domain', docID: 'helpDiskOfferingDomain', dependsOn: 'isPublic', @@ -2140,7 +2199,7 @@ }, isHidden: true }, - zone: { + zoneId: { label: 'label.zone', docID: 'helpDiskOfferingZone', isMultiple: true, @@ -2255,18 +2314,18 @@ } if (args.data.isPublic != "on") { - var domains = (args.data.domain && Array.isArray(args.data.domain)) ? args.data.domain.join(',') : args.data.domain; - if (domains) { + var domainId = (args.data.domainId && Array.isArray(args.data.domainId)) ? args.data.domainId.join(',') : args.data.domainId; + if (domainId) { $.extend(data, { - domainids: domains + domainid: domainId }); } } - var zones = (args.data.zone && Array.isArray(args.data.zone)) ? args.data.zone.join(',') : args.data.zone; - if (zones) { + var zoneId = (args.data.zone && Array.isArray(args.data.zoneId)) ? args.data.zoneId.join(',') : args.data.zoneId; + if (zoneId) { $.extend(data, { - zoneids: zones + zoneid: zoneId }); }