Repository: cloudstack Updated Branches: refs/heads/4.4 936de7e1c -> 1e4a253f7
Handle listAll flag in IAM buildAclSearchParameters. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/1e4a253f Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/1e4a253f Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/1e4a253f Branch: refs/heads/4.4 Commit: 1e4a253f79635a2b2740e1517353a2dbe1f288df Parents: 936de7e1 Author: Min Chen <[email protected]> Authored: Fri Apr 4 11:48:55 2014 -0700 Committer: Min Chen <[email protected]> Committed: Fri Apr 4 11:49:30 2014 -0700 ---------------------------------------------------------------------- .../api/BaseListDomainResourcesCmd.java | 3 +- .../src/com/cloud/user/AccountManagerImpl.java | 71 ++++++++++++-------- test/integration/smoke/test_vm_iam.py | 33 ++++++--- 3 files changed, 66 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1e4a253f/api/src/org/apache/cloudstack/api/BaseListDomainResourcesCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/BaseListDomainResourcesCmd.java b/api/src/org/apache/cloudstack/api/BaseListDomainResourcesCmd.java index 79f7edc..3257d65 100644 --- a/api/src/org/apache/cloudstack/api/BaseListDomainResourcesCmd.java +++ b/api/src/org/apache/cloudstack/api/BaseListDomainResourcesCmd.java @@ -39,7 +39,8 @@ public abstract class BaseListDomainResourcesCmd extends BaseListCmd { } public boolean isRecursive() { - return recursive == null ? false : recursive; + // if listAll is true, recursive is not specified, then recursive should default to true. + return recursive == null ? (listAll() ? true : false) : recursive; } public Long getDomainId() { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1e4a253f/server/src/com/cloud/user/AccountManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index bd7f6a6..d4b6731 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -2164,39 +2164,47 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M public void buildACLSearchParameters(Account caller, Long id, String accountName, Long projectId, List<Long> permittedDomains, List<Long> permittedAccounts, List<Long> permittedResources, Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject, boolean listAll, boolean forProjectInvitation, String action) { - //TODO: need to handle listAll flag + Long domainId = domainIdRecursiveListProject.first(); - if (domainId != null) { - // look for entity in the given domain - Domain domain = _domainDao.findById(domainId); - if (domain == null) { - throw new InvalidParameterValueException("Unable to find domain by id " + domainId); + if (id == null) { + // if id is specified, it will ignore all other parameters + if (domainId != null) { + // look for entity in the given domain + Domain domain = _domainDao.findById(domainId); + if (domain == null) { + throw new InvalidParameterValueException("Unable to find domain by id " + domainId); + } + // check permissions + checkAccess(caller, domain); } - // check permissions - checkAccess(caller, domain); - } - if (accountName != null) { - if (projectId != null) { - throw new InvalidParameterValueException("Account and projectId can't be specified together"); - } + // specific account is specified + if (accountName != null) { + if (projectId != null) { + throw new InvalidParameterValueException("Account and projectId can't be specified together"); + } - Account userAccount = null; - Domain domain = null; - if (domainId != null) { - userAccount = _accountDao.findActiveAccount(accountName, domainId); - domain = _domainDao.findById(domainId); - } else { - userAccount = _accountDao.findActiveAccount(accountName, caller.getDomainId()); - domain = _domainDao.findById(caller.getDomainId()); - } + Account userAccount = null; + Domain domain = null; + if (domainId != null) { + userAccount = _accountDao.findActiveAccount(accountName, domainId); + domain = _domainDao.findById(domainId); + } else { + userAccount = _accountDao.findActiveAccount(accountName, caller.getDomainId()); + domain = _domainDao.findById(caller.getDomainId()); + } - if (userAccount != null) { - //check permissions - checkAccess(caller, null, userAccount); - permittedAccounts.add(userAccount.getId()); - } else { - throw new InvalidParameterValueException("could not find account " + accountName + " in domain " + domain.getUuid()); + if (userAccount != null) { + //check permissions + checkAccess(caller, null, userAccount); + permittedAccounts.add(userAccount.getId()); + } else { + throw new InvalidParameterValueException("could not find account " + accountName + " in domain " + domain.getUuid()); + } + } else if (!listAll && domainId == null) { + // listAll=false with no domain specified will only show caller owned resources + // if domainId is specified, we will filter the list based caller visible resources (owned + granted) + permittedAccounts.add(caller.getId()); } } @@ -2233,7 +2241,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M QuerySelector qs = _querySelectors.get(0); boolean grantedAll = qs.isGrantedAll(caller, action); if ( grantedAll ){ - if ( domainId != null ){ + if (permittedAccounts.isEmpty() && domainId != null) { permittedDomains.add(domainId); } } @@ -2253,6 +2261,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M permittedAccounts.add(acctId); } } + //TODO: we should also filter granted resources based on domainId passed. + // potential bug, if domainId is passed, it may show some granted resources that may not be in that domain. + // to fix this, we need to change the interface to also pass ControlledEntity class to use EntityManager to find + // ControlledEntity instance to check domainId. But this has some issues for those non controlled entities, + // like NetworkACLItem permittedResources.addAll(grantedResources); } } else if (permittedAccounts.isEmpty()) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1e4a253f/test/integration/smoke/test_vm_iam.py ---------------------------------------------------------------------- diff --git a/test/integration/smoke/test_vm_iam.py b/test/integration/smoke/test_vm_iam.py index 80c049b..bdbe75a 100644 --- a/test/integration/smoke/test_vm_iam.py +++ b/test/integration/smoke/test_vm_iam.py @@ -289,7 +289,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1A_apikey self.apiclient.connection.securityKey = self.user_1A_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -312,7 +313,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -336,7 +338,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_2A_apikey self.apiclient.connection.securityKey = self.user_2A_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -378,7 +381,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -426,7 +430,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -479,7 +484,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -522,7 +528,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -563,7 +570,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -610,7 +618,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -653,7 +662,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list), @@ -695,7 +705,8 @@ class TestVMIam(cloudstackTestCase): self.apiclient.connection.apiKey = self.user_1B_apikey self.apiclient.connection.securityKey = self.user_1B_secretkey list_vm_response = list_virtual_machines( - self.apiclient + self.apiclient, + listall=True ) self.assertEqual( isinstance(list_vm_response, list),
