Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id CB97A113CC for ; Tue, 22 Apr 2014 08:02:50 +0000 (UTC) Received: (qmail 65196 invoked by uid 500); 22 Apr 2014 08:02:34 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 65032 invoked by uid 500); 22 Apr 2014 08:02:30 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 64946 invoked by uid 99); 22 Apr 2014 08:02:28 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 22 Apr 2014 08:02:28 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id AB3918398DB; Tue, 22 Apr 2014 08:02:28 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: dahn@apache.org To: commits@cloudstack.apache.org Date: Tue, 22 Apr 2014 08:02:30 -0000 Message-Id: <843dc5b463094b2b927207b7e34d2378@git.apache.org> In-Reply-To: <9a599e6555c64551b291de2c7a3f3ba6@git.apache.org> References: <9a599e6555c64551b291de2c7a3f3ba6@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [03/26] git commit: updated refs/heads/4.4-forward to 0594598 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-forward Commit: bd6a95e8642a06de61c8a0817567df8c1423b602 Parents: 59fa7a9 Author: Min Chen Authored: Thu Apr 17 15:44:35 2014 -0700 Committer: Min Chen 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 permittedResources, Ternary 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 grantedDomains = qs.getAuthorizedDomains(owner, action, accessType); - List grantedAccounts = qs.getAuthorizedAccounts(owner, action, accessType); - List grantedResources = qs.getAuthorizedResources(owner, action, accessType); - - if (domainId != null) { + List grantedDomains = qs.getAuthorizedDomains(caller, action, accessType); + List grantedAccounts = qs.getAuthorizedAccounts(caller, action, accessType); + List 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()) {