Skip to content

Commit 6842583

Browse files
shwstpprshwstppr
authored
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 <abhishek.mrt22@gmail.com>
1 parent 2d0a2e3 commit 6842583

File tree

3 files changed

+166
-25
lines changed

3 files changed

+166
-25
lines changed

‎server/src/main/java/com/cloud/network/NetworkModelImpl.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,7 +1659,7 @@ public void checkCapabilityForProvider(Set<Provider> providers, Service service,
16591659
}
16601660

16611661
@Override
1662-
public void checkNetworkPermissions(Account owner, Network network) {
1662+
public void checkNetworkPermissions(Account caller, Network network) {
16631663
// dahn 20140310: I was thinking of making this an assert but
16641664
// as we hardly ever test with asserts I think
16651665
// we better make sure at runtime.
@@ -1672,25 +1672,25 @@ public void checkNetworkPermissions(Account owner, Network network) {
16721672
if (networkOwner == null)
16731673
throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
16741674
", network does not have an owner");
1675-
if (owner.getType() != Account.Type.PROJECT && networkOwner.getType() == Account.Type.PROJECT) {
1676-
checkProjectNetworkPermissions(owner, networkOwner, network);
1675+
if (!Account.Type.PROJECT.equals(caller.getType()) && Account.Type.PROJECT.equals(networkOwner.getType())) {
1676+
checkProjectNetworkPermissions(caller, networkOwner, network);
16771677
} else {
1678-
List<NetworkVO> networkMap = _networksDao.listBy(owner.getId(), network.getId());
1679-
NetworkPermissionVO networkPermission = _networkPermissionDao.findByNetworkAndAccount(network.getId(), owner.getId());
1678+
List<NetworkVO> networkMap = _networksDao.listBy(caller.getId(), network.getId());
1679+
NetworkPermissionVO networkPermission = _networkPermissionDao.findByNetworkAndAccount(network.getId(), caller.getId());
16801680
if (CollectionUtils.isEmpty(networkMap) && networkPermission == null) {
16811681
throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
16821682
", permission denied");
16831683
}
16841684
}
16851685

16861686
} else {
1687-
if (!isNetworkAvailableInDomain(network.getId(), owner.getDomainId())) {
1688-
DomainVO ownerDomain = _domainDao.findById(owner.getDomainId());
1689-
if (ownerDomain == null) {
1690-
throw new CloudRuntimeException("cannot check permission on account " + owner.getAccountName() + " whose domain does not exist");
1687+
if (!isNetworkAvailableInDomain(network.getId(), caller.getDomainId())) {
1688+
DomainVO callerDomain = _domainDao.findById(caller.getDomainId());
1689+
if (callerDomain == null) {
1690+
throw new CloudRuntimeException("cannot check permission on account " + caller.getAccountName() + " whose domain does not exist");
16911691
}
16921692
throw new PermissionDeniedException("Shared network id=" + ((NetworkVO)network).getUuid() + " is not available in domain id=" +
1693-
ownerDomain.getUuid());
1693+
callerDomain.getUuid());
16941694
}
16951695
}
16961696
}

‎server/src/main/java/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public ApplicationLoadBalancerRule createApplicationLoadBalancer(String name, St
113113
}
114114

115115
Account caller = CallContext.current().getCallingAccount();
116-
_accountMgr.checkAccess(caller, AccessType.UseEntry, false, guestNtwk);
116+
_accountMgr.checkAccess(caller, AccessType.OperateEntry, false, guestNtwk);
117117

118118
Network sourceIpNtwk = _networkModel.getNetwork(sourceIpNetworkId);
119119
if (sourceIpNtwk == null) {

‎server/src/test/java/com/cloud/network/NetworkModelTest.java‎

Lines changed: 155 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,34 +31,49 @@
3131
import java.util.Arrays;
3232
import java.util.Collections;
3333
import java.util.List;
34+
import java.util.Set;
35+
36+
import org.apache.cloudstack.network.NetworkPermissionVO;
37+
import org.apache.cloudstack.network.dao.NetworkPermissionDao;
38+
import org.junit.Before;
39+
import org.junit.Test;
40+
import org.mockito.InjectMocks;
41+
import org.mockito.Mock;
42+
import org.mockito.MockitoAnnotations;
43+
import org.mockito.Spy;
3444

3545
import com.cloud.dc.DataCenter;
3646
import com.cloud.dc.DataCenterVO;
47+
import com.cloud.dc.VlanVO;
3748
import com.cloud.dc.dao.DataCenterDao;
49+
import com.cloud.dc.dao.VlanDao;
50+
import com.cloud.domain.DomainVO;
51+
import com.cloud.domain.dao.DomainDao;
3852
import com.cloud.exception.InvalidParameterValueException;
53+
import com.cloud.exception.PermissionDeniedException;
54+
import com.cloud.network.Network.Provider;
55+
import com.cloud.network.dao.IPAddressDao;
56+
import com.cloud.network.dao.IPAddressVO;
57+
import com.cloud.network.dao.NetworkDao;
58+
import com.cloud.network.dao.NetworkDomainDao;
59+
import com.cloud.network.dao.NetworkDomainVO;
60+
import com.cloud.network.dao.NetworkVO;
3961
import com.cloud.network.dao.PhysicalNetworkDao;
4062
import com.cloud.network.dao.PhysicalNetworkServiceProviderDao;
4163
import com.cloud.network.dao.PhysicalNetworkServiceProviderVO;
4264
import com.cloud.network.dao.PhysicalNetworkVO;
43-
import junit.framework.Assert;
44-
45-
import org.junit.Before;
46-
import org.junit.Test;
47-
48-
import com.cloud.dc.VlanVO;
49-
import com.cloud.dc.dao.VlanDao;
50-
import com.cloud.network.dao.IPAddressDao;
51-
import com.cloud.network.dao.IPAddressVO;
65+
import com.cloud.projects.dao.ProjectDao;
5266
import com.cloud.user.Account;
67+
import com.cloud.user.AccountVO;
68+
import com.cloud.user.DomainManager;
69+
import com.cloud.user.dao.AccountDao;
5370
import com.cloud.utils.db.Filter;
5471
import com.cloud.utils.db.SearchBuilder;
5572
import com.cloud.utils.db.SearchCriteria;
73+
import com.cloud.utils.exception.CloudRuntimeException;
5674
import com.cloud.utils.net.Ip;
57-
import com.cloud.network.Network.Provider;
58-
import org.mockito.InjectMocks;
59-
import org.mockito.Mock;
60-
import org.mockito.MockitoAnnotations;
61-
import org.mockito.Spy;
75+
76+
import junit.framework.Assert;
6277

6378
public class NetworkModelTest {
6479

@@ -85,6 +100,20 @@ public class NetworkModelTest {
85100
private PhysicalNetworkVO physicalNetworkZone2;
86101
@Mock
87102
private PhysicalNetworkServiceProviderVO providerVO;
103+
@Mock
104+
private AccountDao accountDao;
105+
@Mock
106+
private NetworkDao networkDao;
107+
@Mock
108+
private NetworkPermissionDao networkPermissionDao;
109+
@Mock
110+
private NetworkDomainDao networkDomainDao;
111+
@Mock
112+
private DomainManager domainManager;
113+
@Mock
114+
private DomainDao domainDao;
115+
@Mock
116+
private ProjectDao projectDao;
88117

89118
private static final long ZONE_1_ID = 1L;
90119
private static final long ZONE_2_ID = 2L;
@@ -263,4 +292,116 @@ public void checkIp6ParametersTestNullStartAndEndIpv6() {
263292
networkModel.checkIp6Parameters(null, null, IPV6_GATEWAY,IPV6_CIDR);
264293
}
265294

295+
@Test
296+
public void testCheckNetworkPermissions() {
297+
long accountId = 1L;
298+
AccountVO caller = mock(AccountVO.class);
299+
when(caller.getId()).thenReturn(accountId);
300+
when(caller.getType()).thenReturn(Account.Type.NORMAL);
301+
NetworkVO network = mock(NetworkVO.class);
302+
when(network.getGuestType()).thenReturn(Network.GuestType.Isolated);
303+
when(network.getAccountId()).thenReturn(accountId);
304+
when(accountDao.findById(accountId)).thenReturn(caller);
305+
when(networkDao.listBy(caller.getId(), network.getId())).thenReturn(List.of(network));
306+
when(networkPermissionDao.findByNetworkAndAccount(network.getId(), caller.getId())).thenReturn(mock(NetworkPermissionVO.class));
307+
networkModel.checkNetworkPermissions(caller, network);
308+
}
309+
310+
@Test(expected = CloudRuntimeException.class)
311+
public void testCheckNetworkPermissionsNullNetwork() {
312+
AccountVO caller = mock(AccountVO.class);
313+
NetworkVO network = null;
314+
networkModel.checkNetworkPermissions(caller, network);
315+
}
316+
317+
@Test(expected = PermissionDeniedException.class)
318+
public void testCheckNetworkPermissionsNoOwner() {
319+
long accountId = 1L;
320+
AccountVO caller = mock(AccountVO.class);
321+
when(caller.getId()).thenReturn(accountId);
322+
when(caller.getType()).thenReturn(Account.Type.NORMAL);
323+
NetworkVO network = mock(NetworkVO.class);
324+
when(network.getGuestType()).thenReturn(Network.GuestType.Isolated);
325+
when(network.getAccountId()).thenReturn(accountId);
326+
when(accountDao.findById(accountId)).thenReturn(null);
327+
networkModel.checkNetworkPermissions(caller, network);
328+
}
329+
330+
@Test(expected = PermissionDeniedException.class)
331+
public void testCheckNetworkPermissionsNoPermission() {
332+
long accountId = 1L;
333+
AccountVO caller = mock(AccountVO.class);
334+
when(caller.getId()).thenReturn(accountId);
335+
when(caller.getType()).thenReturn(Account.Type.NORMAL);
336+
NetworkVO network = mock(NetworkVO.class);
337+
when(network.getGuestType()).thenReturn(Network.GuestType.Isolated);
338+
when(network.getAccountId()).thenReturn(accountId);
339+
when(accountDao.findById(accountId)).thenReturn(caller);
340+
when(networkDao.listBy(caller.getId(), network.getId())).thenReturn(null);
341+
when(networkPermissionDao.findByNetworkAndAccount(network.getId(), caller.getId())).thenReturn(null);
342+
networkModel.checkNetworkPermissions(caller, network);
343+
}
344+
345+
@Test
346+
public void testCheckNetworkPermissionsSharedNetwork() {
347+
long id = 1L;
348+
long subDomainId = 2L;
349+
AccountVO caller = mock(AccountVO.class);
350+
when(caller.getId()).thenReturn(id);
351+
when(caller.getDomainId()).thenReturn(id);
352+
when(caller.getType()).thenReturn(Account.Type.NORMAL);
353+
NetworkVO network = mock(NetworkVO.class);
354+
when(network.getGuestType()).thenReturn(Network.GuestType.Shared);
355+
when(network.getId()).thenReturn(id);
356+
when(networkDao.findById(network.getId())).thenReturn(network);
357+
NetworkDomainVO networkDomainVO = mock(NetworkDomainVO.class);
358+
when(networkDomainVO.getDomainId()).thenReturn(id);
359+
when(networkDomainDao.getDomainNetworkMapByNetworkId(id)).thenReturn(networkDomainVO);
360+
networkModel.checkNetworkPermissions(caller, network);
361+
when(caller.getDomainId()).thenReturn(subDomainId);
362+
networkDomainVO.subdomainAccess = Boolean.TRUE;
363+
when(domainManager.getDomainParentIds(subDomainId)).thenReturn(Set.of(id));
364+
networkModel.checkNetworkPermissions(caller, network);
365+
}
366+
367+
@Test(expected = PermissionDeniedException.class)
368+
public void testCheckNetworkPermissionsSharedNetworkNoSubDomainAccess() {
369+
long id = 1L;
370+
long subDomainId = 2L;
371+
AccountVO caller = mock(AccountVO.class);
372+
when(caller.getId()).thenReturn(id);
373+
when(caller.getDomainId()).thenReturn(subDomainId);
374+
when(caller.getType()).thenReturn(Account.Type.NORMAL);
375+
NetworkVO network = mock(NetworkVO.class);
376+
when(network.getGuestType()).thenReturn(Network.GuestType.Shared);
377+
when(network.getId()).thenReturn(id);
378+
when(networkDao.findById(network.getId())).thenReturn(network);
379+
when(domainDao.findById(caller.getDomainId())).thenReturn(mock(DomainVO.class));
380+
NetworkDomainVO networkDomainVO = mock(NetworkDomainVO.class);
381+
when(networkDomainVO.getDomainId()).thenReturn(id);
382+
networkDomainVO.subdomainAccess = Boolean.FALSE;
383+
when(networkDomainDao.getDomainNetworkMapByNetworkId(id)).thenReturn(networkDomainVO);
384+
networkModel.checkNetworkPermissions(caller, network);
385+
}
386+
387+
@Test(expected = PermissionDeniedException.class)
388+
public void testCheckNetworkPermissionsSharedNetworkNotSubDomain() {
389+
long id = 1L;
390+
long subDomainId = 2L;
391+
AccountVO caller = mock(AccountVO.class);
392+
when(caller.getId()).thenReturn(id);
393+
when(caller.getDomainId()).thenReturn(subDomainId);
394+
when(caller.getType()).thenReturn(Account.Type.NORMAL);
395+
NetworkVO network = mock(NetworkVO.class);
396+
when(network.getGuestType()).thenReturn(Network.GuestType.Shared);
397+
when(network.getId()).thenReturn(id);
398+
when(networkDao.findById(network.getId())).thenReturn(network);
399+
when(domainDao.findById(caller.getDomainId())).thenReturn(mock(DomainVO.class));
400+
NetworkDomainVO networkDomainVO = mock(NetworkDomainVO.class);
401+
when(networkDomainVO.getDomainId()).thenReturn(id);
402+
networkDomainVO.subdomainAccess = Boolean.TRUE;
403+
when(networkDomainDao.getDomainNetworkMapByNetworkId(id)).thenReturn(networkDomainVO);
404+
when(domainManager.getDomainParentIds(subDomainId)).thenReturn(Set.of(0L));
405+
networkModel.checkNetworkPermissions(caller, network);
406+
}
266407
}

0 commit comments

Comments
 (0)