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/18 00:56:16 UTC
git commit: updated refs/heads/4.4 to bd6a95e
Repository: cloudstack
Updated Branches:
refs/heads/4.4 59fa7a9c1 -> bd6a95e86
Fix IAM list api implementation based on agreed interpretation for
listAll, isRecursive, domainId and account.
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/bd6a95e8
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/bd6a95e8
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/bd6a95e8
Branch: refs/heads/4.4
Commit: bd6a95e8642a06de61c8a0817567df8c1423b602
Parents: 59fa7a9
Author: Min Chen <mi...@citrix.com>
Authored: Thu Apr 17 15:44:35 2014 -0700
Committer: Min Chen <mi...@citrix.com>
Committed: Thu Apr 17 15:46:29 2014 -0700
----------------------------------------------------------------------
.../com/cloud/api/query/QueryManagerImpl.java | 2 +-
.../src/com/cloud/user/AccountManagerImpl.java | 44 +++++++++++++-------
2 files changed, 31 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/bd6a95e8/server/src/com/cloud/api/query/QueryManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java
index 249e90e..5fb8a3e 100644
--- a/server/src/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/com/cloud/api/query/QueryManagerImpl.java
@@ -685,7 +685,7 @@ public class QueryManagerImpl extends ManagerBase implements QueryService {
cmd.getDomainId(), cmd.isRecursive(), null);
_accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedDomains, permittedAccounts, permittedResources,
domainIdRecursiveListProject, cmd.listAll(), false, "listInstanceGroups");
- Long domainId = domainIdRecursiveListProject.first();
+ // Long domainId = domainIdRecursiveListProject.first();
Boolean isRecursive = domainIdRecursiveListProject.second();
ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/bd6a95e8/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 cabef40..364d0b7 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -2188,8 +2188,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
List<Long> permittedResources, Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject, boolean listAll, boolean forProjectInvitation,
String action) {
- Account owner = null; // for impersonation
Long domainId = domainIdRecursiveListProject.first();
+ Long accountId = null;
if (id == null) {
// if id is specified, it will ignore all other parameters
if (domainId != null) {
@@ -2202,7 +2202,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
checkAccess(caller, domain);
}
- // specific account is specified, we need to impersonate that account instead of caller
+ // specific account is specified, we need to filter contents to only show contents owned by that account.
if (accountName != null) {
if (projectId != null) {
throw new InvalidParameterValueException("Account and projectId can't be specified together");
@@ -2221,7 +2221,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
if (userAccount != null) {
//check permissions
checkAccess(caller, null, userAccount);
- owner = userAccount;
+ accountId = userAccount.getId();
} else {
throw new InvalidParameterValueException("could not find account " + accountName + " in domain " + domain.getUuid());
}
@@ -2249,10 +2249,6 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
}
} else {
- if (owner == null) {
- // no impersonation, so we directly check permission for caller
- owner = caller;
- }
AccessType accessType = AccessType.UseEntry;
if (listAll || id != null) {
// listAll = true or id given should show all resources that owner has ListEntry access type.
@@ -2268,18 +2264,37 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
return; // no futher filtering
QuerySelector qs = _querySelectors.get(0);
- boolean grantedAll = qs.isGrantedAll(owner, action, accessType);
+ boolean grantedAll = qs.isGrantedAll(caller, action, accessType);
+
if ( grantedAll ){
- if (domainId != null) {
+ if (accountId != null) {
+ permittedAccounts.add(accountId);
+ domainIdRecursiveListProject.second(false); // isRecursive is only valid if only domainId is passed.
+ } else if (domainId != null) {
permittedDomains.add(domainId);
+ } else {
+ domainIdRecursiveListProject.second(false); // isRecursive is only valid if only domainId is passed.
}
}
else {
- List<Long> grantedDomains = qs.getAuthorizedDomains(owner, action, accessType);
- List<Long> grantedAccounts = qs.getAuthorizedAccounts(owner, action, accessType);
- List<Long> grantedResources = qs.getAuthorizedResources(owner, action, accessType);
-
- if (domainId != null) {
+ List<Long> grantedDomains = qs.getAuthorizedDomains(caller, action, accessType);
+ List<Long> grantedAccounts = qs.getAuthorizedAccounts(caller, action, accessType);
+ List<Long> grantedResources = qs.getAuthorizedResources(caller, action, accessType);
+
+ if (accountId != null) {
+ // specific account filter is specified
+ if (grantedAccounts.contains(accountId)) {
+ permittedAccounts.add(accountId);
+ } else {
+ //TODO: we should also filter granted resources based on accountId passed.
+ // potential bug, if accountId is passed, it may show some granted resources that may not be owned by that account.
+ // to fix this, we need to change the interface to also pass ControlledEntity class to use EntityManager to find
+ // ControlledEntity instance to check accountId. But this has some issues for those non controlled entities,
+ // like NetworkACLItem
+ permittedResources.addAll(grantedResources);
+ }
+ domainIdRecursiveListProject.second(false); // isRecursive is only valid if only domainId is passed.
+ } else if (domainId != null) {
// specific domain and no account is specified
if (grantedDomains.contains(domainId)) {
permittedDomains.add(domainId);
@@ -2302,6 +2317,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
permittedDomains.addAll(grantedDomains);
permittedAccounts.addAll(grantedAccounts);
permittedResources.addAll(grantedResources);
+ domainIdRecursiveListProject.second(false); // isRecursive is only valid if only domainId is passed.
}
if (permittedDomains.isEmpty() && permittedAccounts.isEmpty() & permittedResources.isEmpty()) {