You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mc...@apache.org on 2014/04/04 20:58:31 UTC
git commit: updated refs/heads/4.4 to 1e4a253
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 <mi...@citrix.com>
Authored: Fri Apr 4 11:48:55 2014 -0700
Committer: Min Chen <mi...@citrix.com>
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),