diff --git a/api/src/com/cloud/network/vpc/NetworkACLService.java b/api/src/com/cloud/network/vpc/NetworkACLService.java index ec53c26a4ce..d2ff287da79 100644 --- a/api/src/com/cloud/network/vpc/NetworkACLService.java +++ b/api/src/com/cloud/network/vpc/NetworkACLService.java @@ -20,6 +20,7 @@ package com.cloud.network.vpc; import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.Pair; import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd; +import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd; import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd; import java.util.List; @@ -43,13 +44,10 @@ public interface NetworkACLService { /** * List NetworkACLs by Id/Name/Network or Vpc it belongs to - * @param id - * @param name - * @param networkId - * @param vpcId + * @param cmd * @return */ - Pair,Integer> listNetworkACLs(Long id, String name, Long networkId, Long vpcId); + Pair,Integer> listNetworkACLs(ListNetworkACLListsCmd cmd); /** * Delete specified network ACL. Deletion fails if the list is not empty diff --git a/api/src/org/apache/cloudstack/api/command/user/network/ListNetworkACLListsCmd.java b/api/src/org/apache/cloudstack/api/command/user/network/ListNetworkACLListsCmd.java index bb825d9f9f9..abd614368a0 100644 --- a/api/src/org/apache/cloudstack/api/command/user/network/ListNetworkACLListsCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/network/ListNetworkACLListsCmd.java @@ -21,6 +21,7 @@ import com.cloud.utils.Pair; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListCmd; +import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.NetworkACLResponse; @@ -32,7 +33,7 @@ import java.util.ArrayList; import java.util.List; @APICommand(name = "listNetworkACLLists", description="Lists all network ACLs", responseObject=NetworkACLResponse.class) -public class ListNetworkACLListsCmd extends BaseListCmd { +public class ListNetworkACLListsCmd extends BaseListProjectAndAccountResourcesCmd { public static final Logger s_logger = Logger.getLogger(ListNetworkACLListsCmd.class.getName()); private static final String s_name = "listnetworkacllistsresponse"; @@ -87,7 +88,7 @@ public class ListNetworkACLListsCmd extends BaseListCmd { @Override public void execute(){ - Pair,Integer> result = _networkACLService.listNetworkACLs(getId(), getName(), getNetworkId(), getVpcId()); + Pair,Integer> result = _networkACLService.listNetworkACLs(this); ListResponse response = new ListResponse(); List aclResponses = new ArrayList(); diff --git a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java index d12e57701cc..b925c8bf703 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java @@ -23,6 +23,8 @@ import java.util.Map; import javax.ejb.Local; import javax.inject.Inject; +import com.cloud.network.vpc.dao.VpcDao; +import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -88,6 +90,8 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ VpcManager _vpcMgr; @Inject EntityManager _entityMgr; + @Inject + VpcDao _vpcDao; @Override public NetworkACL createNetworkACL(String name, String description, long vpcId) { @@ -106,12 +110,18 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ } @Override - public Pair, Integer> listNetworkACLs(Long id, String name, Long networkId, Long vpcId) { + public Pair, Integer> listNetworkACLs(ListNetworkACLListsCmd cmd) { + Long id = cmd.getId(); + String name = cmd.getName(); + Long networkId = cmd.getNetworkId(); + Long vpcId = cmd.getVpcId(); SearchBuilder sb = _networkACLDao.createSearchBuilder(); sb.and("id", sb.entity().getId(), Op.EQ); sb.and("name", sb.entity().getName(), Op.EQ); sb.and("vpcId", sb.entity().getVpcId(), Op.IN); + Account caller = CallContext.current().getCallingAccount(); + if(networkId != null){ SearchBuilder network = _networkDao.createSearchBuilder(); network.and("networkId", network.entity().getId(), Op.EQ); @@ -128,8 +138,43 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ } if(vpcId != null){ + Vpc vpc = _entityMgr.findById(Vpc.class, vpcId); + if(vpc == null){ + throw new InvalidParameterValueException("Unable to find VPC"); + } + _accountMgr.checkAccess(caller, null, true, vpc); //Include vpcId 0 to list default ACLs sc.setParameters("vpcId", vpcId, 0); + } else { + //ToDo: Add accountId to network_acl table for permission check + + // VpcId is not specified. Find permitted VPCs for the caller + // and list ACLs belonging to the permitted VPCs + List permittedAccounts = new ArrayList(); + Long domainId = cmd.getDomainId(); + boolean isRecursive = cmd.isRecursive(); + String accountName = cmd.getAccountName(); + Long projectId = cmd.getProjectId(); + boolean listAll = cmd.listAll(); + Ternary domainIdRecursiveListProject = new Ternary(domainId, isRecursive, null); + _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, + listAll, false); + domainId = domainIdRecursiveListProject.first(); + isRecursive = domainIdRecursiveListProject.second(); + ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + SearchBuilder sbVpc = _vpcDao.createSearchBuilder(); + _accountMgr.buildACLSearchBuilder(sbVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria scVpc = sbVpc.create(); + _accountMgr.buildACLSearchCriteria(scVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + List vpcs = _vpcDao.search(scVpc, null); + List vpcIds = new ArrayList(); + for (VpcVO vpc : vpcs) { + vpcIds.add(vpc.getId()); + } + //Add vpc_id 0 to list default ACLs + vpcIds.add(0L); + sc.setParameters("vpcId", vpcIds.toArray()); } if(networkId != null){ @@ -420,21 +465,10 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ String protocol = cmd.getProtocol(); String action = cmd.getAction(); Map tags = cmd.getTags(); - Account caller = CallContext.current().getCallingAccount(); - List permittedAccounts = new ArrayList(); - - Ternary domainIdRecursiveListProject = - new Ternary(cmd.getDomainId(), cmd.isRecursive(), null); - _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedAccounts, - domainIdRecursiveListProject, cmd.listAll(), false); - Long domainId = domainIdRecursiveListProject.first(); - Boolean isRecursive = domainIdRecursiveListProject.second(); - ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); Filter filter = new Filter(NetworkACLItemVO.class, "id", false, cmd.getStartIndex(), cmd.getPageSizeVal()); SearchBuilder sb = _networkACLItemDao.createSearchBuilder(); - //_accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("id", sb.entity().getId(), Op.EQ); sb.and("aclId", sb.entity().getAclId(), Op.EQ); @@ -454,8 +488,14 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ sb.join("tagSearch", tagSearch, sb.entity().getId(), tagSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER); } + if(aclId == null){ + //Join with network_acl table when aclId is not specified to list acl_items within permitted VPCs + SearchBuilder vpcSearch = _networkACLDao.createSearchBuilder(); + vpcSearch.and("vpcId", vpcSearch.entity().getVpcId(), Op.IN); + sb.join("vpcSearch", vpcSearch, sb.entity().getAclId(), vpcSearch.entity().getId(), JoinBuilder.JoinType.INNER); + } + SearchCriteria sc = sb.create(); - // _accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); if (id != null) { sc.setParameters("id", id); @@ -471,7 +511,47 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ } if(aclId != null){ + // Get VPC and check access + NetworkACL acl = _networkACLDao.findById(aclId); + if(acl.getVpcId() != 0){ + Vpc vpc = _vpcDao.findById(acl.getVpcId()); + if(vpc == null){ + throw new InvalidParameterValueException("Unable to find VPC associated with acl"); + } + _accountMgr.checkAccess(caller, null, true, vpc); + } sc.setParameters("aclId", aclId); + } else { + //ToDo: Add accountId to network_acl_item table for permission check + + + // aclId is not specified + // List permitted VPCs and filter aclItems + List permittedAccounts = new ArrayList(); + Long domainId = cmd.getDomainId(); + boolean isRecursive = cmd.isRecursive(); + String accountName = cmd.getAccountName(); + Long projectId = cmd.getProjectId(); + boolean listAll = cmd.listAll(); + Ternary domainIdRecursiveListProject = new Ternary(domainId, isRecursive, null); + _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, + listAll, false); + domainId = domainIdRecursiveListProject.first(); + isRecursive = domainIdRecursiveListProject.second(); + ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + SearchBuilder sbVpc = _vpcDao.createSearchBuilder(); + _accountMgr.buildACLSearchBuilder(sbVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria scVpc = sbVpc.create(); + _accountMgr.buildACLSearchCriteria(scVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + List vpcs = _vpcDao.search(scVpc, null); + List vpcIds = new ArrayList(); + for (VpcVO vpc : vpcs) { + vpcIds.add(vpc.getId()); + } + //Add vpc_id 0 to list acl_items in default ACL + vpcIds.add(0L); + sc.setJoinParameters("vpcSearch", "vpcId", vpcIds.toArray()); } if(protocol != null){ diff --git a/server/test/com/cloud/vpc/NetworkACLServiceTest.java b/server/test/com/cloud/vpc/NetworkACLServiceTest.java index 1660e011458..41a7e3202bc 100644 --- a/server/test/com/cloud/vpc/NetworkACLServiceTest.java +++ b/server/test/com/cloud/vpc/NetworkACLServiceTest.java @@ -20,6 +20,7 @@ import java.util.UUID; import javax.inject.Inject; +import com.cloud.network.vpc.dao.VpcDao; import junit.framework.TestCase; import org.apache.log4j.Logger; @@ -85,6 +86,8 @@ public class NetworkACLServiceTest extends TestCase{ NetworkACLItemDao _networkACLItemDao; @Inject EntityManager _entityMgr; + @Inject + VpcDao _vpcDao; private CreateNetworkACLCmd createACLItemCmd; private NetworkACLVO acl; @@ -245,6 +248,11 @@ public class NetworkACLServiceTest extends TestCase{ return Mockito.mock(VpcGatewayDao.class); } + @Bean + public VpcDao vpcDao () { + return Mockito.mock(VpcDao.class); + } + public static class Library implements TypeFilter { @Override