Updated Branches: refs/heads/4.3 391395f14 -> 33ff20e1c
CLOUDSTACK-5145 : Added permission checks while listing network ACLs and acl Items. Users will be able to list items that they have access to. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/33ff20e1 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/33ff20e1 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/33ff20e1 Branch: refs/heads/4.3 Commit: 33ff20e1c31b18d7618f04b672cce0b8f110d7dc Parents: 391395f Author: Kishan Kavala <[email protected]> Authored: Mon Dec 9 19:49:17 2013 +0530 Committer: Kishan Kavala <[email protected]> Committed: Mon Dec 9 19:49:29 2013 +0530 ---------------------------------------------------------------------- .../cloud/network/vpc/NetworkACLService.java | 8 +- .../user/network/ListNetworkACLListsCmd.java | 5 +- .../network/vpc/NetworkACLServiceImpl.java | 106 ++++++++++++++++--- .../com/cloud/vpc/NetworkACLServiceTest.java | 8 ++ 4 files changed, 107 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33ff20e1/api/src/com/cloud/network/vpc/NetworkACLService.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/vpc/NetworkACLService.java b/api/src/com/cloud/network/vpc/NetworkACLService.java index ec53c26..d2ff287 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<List<? extends NetworkACL>,Integer> listNetworkACLs(Long id, String name, Long networkId, Long vpcId); + Pair<List<? extends NetworkACL>,Integer> listNetworkACLs(ListNetworkACLListsCmd cmd); /** * Delete specified network ACL. Deletion fails if the list is not empty http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33ff20e1/api/src/org/apache/cloudstack/api/command/user/network/ListNetworkACLListsCmd.java ---------------------------------------------------------------------- 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 bb825d9..abd6143 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<List<? extends NetworkACL>,Integer> result = _networkACLService.listNetworkACLs(getId(), getName(), getNetworkId(), getVpcId()); + Pair<List<? extends NetworkACL>,Integer> result = _networkACLService.listNetworkACLs(this); ListResponse<NetworkACLResponse> response = new ListResponse<NetworkACLResponse>(); List<NetworkACLResponse> aclResponses = new ArrayList<NetworkACLResponse>(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33ff20e1/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java index d12e577..b925c8b 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<List<? extends NetworkACL>, Integer> listNetworkACLs(Long id, String name, Long networkId, Long vpcId) { + public Pair<List<? extends NetworkACL>, Integer> listNetworkACLs(ListNetworkACLListsCmd cmd) { + Long id = cmd.getId(); + String name = cmd.getName(); + Long networkId = cmd.getNetworkId(); + Long vpcId = cmd.getVpcId(); SearchBuilder<NetworkACLVO> 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<NetworkVO> 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<Long> permittedAccounts = new ArrayList<Long>(); + Long domainId = cmd.getDomainId(); + boolean isRecursive = cmd.isRecursive(); + String accountName = cmd.getAccountName(); + Long projectId = cmd.getProjectId(); + boolean listAll = cmd.listAll(); + Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, + ListProjectResourcesCriteria>(domainId, isRecursive, null); + _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, + listAll, false); + domainId = domainIdRecursiveListProject.first(); + isRecursive = domainIdRecursiveListProject.second(); + ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + SearchBuilder<VpcVO> sbVpc = _vpcDao.createSearchBuilder(); + _accountMgr.buildACLSearchBuilder(sbVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria<VpcVO> scVpc = sbVpc.create(); + _accountMgr.buildACLSearchCriteria(scVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + List<VpcVO> vpcs = _vpcDao.search(scVpc, null); + List<Long> vpcIds = new ArrayList<Long>(); + 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<String, String> tags = cmd.getTags(); - Account caller = CallContext.current().getCallingAccount(); - List<Long> permittedAccounts = new ArrayList<Long>(); - - Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = - new Ternary<Long, Boolean, ListProjectResourcesCriteria>(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<NetworkACLItemVO> 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<NetworkACLVO> 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<NetworkACLItemVO> 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<Long> permittedAccounts = new ArrayList<Long>(); + Long domainId = cmd.getDomainId(); + boolean isRecursive = cmd.isRecursive(); + String accountName = cmd.getAccountName(); + Long projectId = cmd.getProjectId(); + boolean listAll = cmd.listAll(); + Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, + ListProjectResourcesCriteria>(domainId, isRecursive, null); + _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, + listAll, false); + domainId = domainIdRecursiveListProject.first(); + isRecursive = domainIdRecursiveListProject.second(); + ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + SearchBuilder<VpcVO> sbVpc = _vpcDao.createSearchBuilder(); + _accountMgr.buildACLSearchBuilder(sbVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria<VpcVO> scVpc = sbVpc.create(); + _accountMgr.buildACLSearchCriteria(scVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + List<VpcVO> vpcs = _vpcDao.search(scVpc, null); + List<Long> vpcIds = new ArrayList<Long>(); + 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){ http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33ff20e1/server/test/com/cloud/vpc/NetworkACLServiceTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/vpc/NetworkACLServiceTest.java b/server/test/com/cloud/vpc/NetworkACLServiceTest.java index 1660e01..41a7e32 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
