You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2022/08/09 11:06:50 UTC
[cloudstack] branch 4.17 updated: vpc,network: fix createLoadBalancer access on user network (#6591)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch 4.17
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.17 by this push:
new 6842583034 vpc,network: fix createLoadBalancer access on user network (#6591)
6842583034 is described below
commit 68425830340de523e5310d6788cdbb0e889ad9f2
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Tue Aug 9 16:36:39 2022 +0530
vpc,network: fix createLoadBalancer access on user network (#6591)
While checking network access for creating load-balancer use AccessType.OperateEntry
Refactor variable name in NetworkModelImpl::checkNetworkPermissions
Fixes: #6590
Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
.../java/com/cloud/network/NetworkModelImpl.java | 20 +--
.../lb/ApplicationLoadBalancerManagerImpl.java | 2 +-
.../java/com/cloud/network/NetworkModelTest.java | 169 +++++++++++++++++++--
3 files changed, 166 insertions(+), 25 deletions(-)
diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
index 66d79cc350..c601c13e6a 100644
--- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java
+++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
@@ -1659,7 +1659,7 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
}
@Override
- public void checkNetworkPermissions(Account owner, Network network) {
+ public void checkNetworkPermissions(Account caller, Network network) {
// dahn 20140310: I was thinking of making this an assert but
// as we hardly ever test with asserts I think
// we better make sure at runtime.
@@ -1672,11 +1672,11 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
if (networkOwner == null)
throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
", network does not have an owner");
- if (owner.getType() != Account.Type.PROJECT && networkOwner.getType() == Account.Type.PROJECT) {
- checkProjectNetworkPermissions(owner, networkOwner, network);
+ if (!Account.Type.PROJECT.equals(caller.getType()) && Account.Type.PROJECT.equals(networkOwner.getType())) {
+ checkProjectNetworkPermissions(caller, networkOwner, network);
} else {
- List<NetworkVO> networkMap = _networksDao.listBy(owner.getId(), network.getId());
- NetworkPermissionVO networkPermission = _networkPermissionDao.findByNetworkAndAccount(network.getId(), owner.getId());
+ List<NetworkVO> networkMap = _networksDao.listBy(caller.getId(), network.getId());
+ NetworkPermissionVO networkPermission = _networkPermissionDao.findByNetworkAndAccount(network.getId(), caller.getId());
if (CollectionUtils.isEmpty(networkMap) && networkPermission == null) {
throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
", permission denied");
@@ -1684,13 +1684,13 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
}
} else {
- if (!isNetworkAvailableInDomain(network.getId(), owner.getDomainId())) {
- DomainVO ownerDomain = _domainDao.findById(owner.getDomainId());
- if (ownerDomain == null) {
- throw new CloudRuntimeException("cannot check permission on account " + owner.getAccountName() + " whose domain does not exist");
+ if (!isNetworkAvailableInDomain(network.getId(), caller.getDomainId())) {
+ DomainVO callerDomain = _domainDao.findById(caller.getDomainId());
+ if (callerDomain == null) {
+ throw new CloudRuntimeException("cannot check permission on account " + caller.getAccountName() + " whose domain does not exist");
}
throw new PermissionDeniedException("Shared network id=" + ((NetworkVO)network).getUuid() + " is not available in domain id=" +
- ownerDomain.getUuid());
+ callerDomain.getUuid());
}
}
}
diff --git a/server/src/main/java/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java
index 1c9af7a334..d9f1db616f 100644
--- a/server/src/main/java/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java
@@ -113,7 +113,7 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A
}
Account caller = CallContext.current().getCallingAccount();
- _accountMgr.checkAccess(caller, AccessType.UseEntry, false, guestNtwk);
+ _accountMgr.checkAccess(caller, AccessType.OperateEntry, false, guestNtwk);
Network sourceIpNtwk = _networkModel.getNetwork(sourceIpNetworkId);
if (sourceIpNtwk == null) {
diff --git a/server/src/test/java/com/cloud/network/NetworkModelTest.java b/server/src/test/java/com/cloud/network/NetworkModelTest.java
index af14cbd7ad..b52335035f 100644
--- a/server/src/test/java/com/cloud/network/NetworkModelTest.java
+++ b/server/src/test/java/com/cloud/network/NetworkModelTest.java
@@ -31,34 +31,49 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.Set;
+
+import org.apache.cloudstack.network.NetworkPermissionVO;
+import org.apache.cloudstack.network.dao.NetworkPermissionDao;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.VlanVO;
import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.dc.dao.VlanDao;
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
+import com.cloud.network.Network.Provider;
+import com.cloud.network.dao.IPAddressDao;
+import com.cloud.network.dao.IPAddressVO;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkDomainDao;
+import com.cloud.network.dao.NetworkDomainVO;
+import com.cloud.network.dao.NetworkVO;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkServiceProviderDao;
import com.cloud.network.dao.PhysicalNetworkServiceProviderVO;
import com.cloud.network.dao.PhysicalNetworkVO;
-import junit.framework.Assert;
-
-import org.junit.Before;
-import org.junit.Test;
-
-import com.cloud.dc.VlanVO;
-import com.cloud.dc.dao.VlanDao;
-import com.cloud.network.dao.IPAddressDao;
-import com.cloud.network.dao.IPAddressVO;
+import com.cloud.projects.dao.ProjectDao;
import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.DomainManager;
+import com.cloud.user.dao.AccountDao;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.net.Ip;
-import com.cloud.network.Network.Provider;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
-import org.mockito.Spy;
+
+import junit.framework.Assert;
public class NetworkModelTest {
@@ -85,6 +100,20 @@ public class NetworkModelTest {
private PhysicalNetworkVO physicalNetworkZone2;
@Mock
private PhysicalNetworkServiceProviderVO providerVO;
+ @Mock
+ private AccountDao accountDao;
+ @Mock
+ private NetworkDao networkDao;
+ @Mock
+ private NetworkPermissionDao networkPermissionDao;
+ @Mock
+ private NetworkDomainDao networkDomainDao;
+ @Mock
+ private DomainManager domainManager;
+ @Mock
+ private DomainDao domainDao;
+ @Mock
+ private ProjectDao projectDao;
private static final long ZONE_1_ID = 1L;
private static final long ZONE_2_ID = 2L;
@@ -263,4 +292,116 @@ public class NetworkModelTest {
networkModel.checkIp6Parameters(null, null, IPV6_GATEWAY,IPV6_CIDR);
}
+ @Test
+ public void testCheckNetworkPermissions() {
+ long accountId = 1L;
+ AccountVO caller = mock(AccountVO.class);
+ when(caller.getId()).thenReturn(accountId);
+ when(caller.getType()).thenReturn(Account.Type.NORMAL);
+ NetworkVO network = mock(NetworkVO.class);
+ when(network.getGuestType()).thenReturn(Network.GuestType.Isolated);
+ when(network.getAccountId()).thenReturn(accountId);
+ when(accountDao.findById(accountId)).thenReturn(caller);
+ when(networkDao.listBy(caller.getId(), network.getId())).thenReturn(List.of(network));
+ when(networkPermissionDao.findByNetworkAndAccount(network.getId(), caller.getId())).thenReturn(mock(NetworkPermissionVO.class));
+ networkModel.checkNetworkPermissions(caller, network);
+ }
+
+ @Test(expected = CloudRuntimeException.class)
+ public void testCheckNetworkPermissionsNullNetwork() {
+ AccountVO caller = mock(AccountVO.class);
+ NetworkVO network = null;
+ networkModel.checkNetworkPermissions(caller, network);
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testCheckNetworkPermissionsNoOwner() {
+ long accountId = 1L;
+ AccountVO caller = mock(AccountVO.class);
+ when(caller.getId()).thenReturn(accountId);
+ when(caller.getType()).thenReturn(Account.Type.NORMAL);
+ NetworkVO network = mock(NetworkVO.class);
+ when(network.getGuestType()).thenReturn(Network.GuestType.Isolated);
+ when(network.getAccountId()).thenReturn(accountId);
+ when(accountDao.findById(accountId)).thenReturn(null);
+ networkModel.checkNetworkPermissions(caller, network);
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testCheckNetworkPermissionsNoPermission() {
+ long accountId = 1L;
+ AccountVO caller = mock(AccountVO.class);
+ when(caller.getId()).thenReturn(accountId);
+ when(caller.getType()).thenReturn(Account.Type.NORMAL);
+ NetworkVO network = mock(NetworkVO.class);
+ when(network.getGuestType()).thenReturn(Network.GuestType.Isolated);
+ when(network.getAccountId()).thenReturn(accountId);
+ when(accountDao.findById(accountId)).thenReturn(caller);
+ when(networkDao.listBy(caller.getId(), network.getId())).thenReturn(null);
+ when(networkPermissionDao.findByNetworkAndAccount(network.getId(), caller.getId())).thenReturn(null);
+ networkModel.checkNetworkPermissions(caller, network);
+ }
+
+ @Test
+ public void testCheckNetworkPermissionsSharedNetwork() {
+ long id = 1L;
+ long subDomainId = 2L;
+ AccountVO caller = mock(AccountVO.class);
+ when(caller.getId()).thenReturn(id);
+ when(caller.getDomainId()).thenReturn(id);
+ when(caller.getType()).thenReturn(Account.Type.NORMAL);
+ NetworkVO network = mock(NetworkVO.class);
+ when(network.getGuestType()).thenReturn(Network.GuestType.Shared);
+ when(network.getId()).thenReturn(id);
+ when(networkDao.findById(network.getId())).thenReturn(network);
+ NetworkDomainVO networkDomainVO = mock(NetworkDomainVO.class);
+ when(networkDomainVO.getDomainId()).thenReturn(id);
+ when(networkDomainDao.getDomainNetworkMapByNetworkId(id)).thenReturn(networkDomainVO);
+ networkModel.checkNetworkPermissions(caller, network);
+ when(caller.getDomainId()).thenReturn(subDomainId);
+ networkDomainVO.subdomainAccess = Boolean.TRUE;
+ when(domainManager.getDomainParentIds(subDomainId)).thenReturn(Set.of(id));
+ networkModel.checkNetworkPermissions(caller, network);
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testCheckNetworkPermissionsSharedNetworkNoSubDomainAccess() {
+ long id = 1L;
+ long subDomainId = 2L;
+ AccountVO caller = mock(AccountVO.class);
+ when(caller.getId()).thenReturn(id);
+ when(caller.getDomainId()).thenReturn(subDomainId);
+ when(caller.getType()).thenReturn(Account.Type.NORMAL);
+ NetworkVO network = mock(NetworkVO.class);
+ when(network.getGuestType()).thenReturn(Network.GuestType.Shared);
+ when(network.getId()).thenReturn(id);
+ when(networkDao.findById(network.getId())).thenReturn(network);
+ when(domainDao.findById(caller.getDomainId())).thenReturn(mock(DomainVO.class));
+ NetworkDomainVO networkDomainVO = mock(NetworkDomainVO.class);
+ when(networkDomainVO.getDomainId()).thenReturn(id);
+ networkDomainVO.subdomainAccess = Boolean.FALSE;
+ when(networkDomainDao.getDomainNetworkMapByNetworkId(id)).thenReturn(networkDomainVO);
+ networkModel.checkNetworkPermissions(caller, network);
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testCheckNetworkPermissionsSharedNetworkNotSubDomain() {
+ long id = 1L;
+ long subDomainId = 2L;
+ AccountVO caller = mock(AccountVO.class);
+ when(caller.getId()).thenReturn(id);
+ when(caller.getDomainId()).thenReturn(subDomainId);
+ when(caller.getType()).thenReturn(Account.Type.NORMAL);
+ NetworkVO network = mock(NetworkVO.class);
+ when(network.getGuestType()).thenReturn(Network.GuestType.Shared);
+ when(network.getId()).thenReturn(id);
+ when(networkDao.findById(network.getId())).thenReturn(network);
+ when(domainDao.findById(caller.getDomainId())).thenReturn(mock(DomainVO.class));
+ NetworkDomainVO networkDomainVO = mock(NetworkDomainVO.class);
+ when(networkDomainVO.getDomainId()).thenReturn(id);
+ networkDomainVO.subdomainAccess = Boolean.TRUE;
+ when(networkDomainDao.getDomainNetworkMapByNetworkId(id)).thenReturn(networkDomainVO);
+ when(domainManager.getDomainParentIds(subDomainId)).thenReturn(Set.of(0L));
+ networkModel.checkNetworkPermissions(caller, network);
+ }
}