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 2013/12/21 02:58:31 UTC

git commit: updated refs/heads/rbac to 630b7fb

Updated Branches:
  refs/heads/rbac 4bd2df42d -> 630b7fb4a


Fix listTemplates issues with new ACL model.

Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/630b7fb4
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/630b7fb4
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/630b7fb4

Branch: refs/heads/rbac
Commit: 630b7fb4a1e715a2da2f81465383d225f948bc44
Parents: 4bd2df4
Author: Min Chen <mi...@citrix.com>
Authored: Fri Dec 20 17:57:44 2013 -0800
Committer: Min Chen <mi...@citrix.com>
Committed: Fri Dec 20 17:57:44 2013 -0800

----------------------------------------------------------------------
 .../org/apache/cloudstack/acl/AclService.java   |   2 +
 .../com/cloud/api/query/QueryManagerImpl.java   | 156 +++++++++----------
 .../src/com/cloud/user/AccountManagerImpl.java  |  54 ++++---
 .../apache/cloudstack/acl/AclServiceImpl.java   |  16 +-
 4 files changed, 126 insertions(+), 102 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/630b7fb4/api/src/org/apache/cloudstack/acl/AclService.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/acl/AclService.java b/api/src/org/apache/cloudstack/acl/AclService.java
index c8d8b48..b6faf1a 100644
--- a/api/src/org/apache/cloudstack/acl/AclService.java
+++ b/api/src/org/apache/cloudstack/acl/AclService.java
@@ -57,6 +57,8 @@ public interface AclService {
     List<AclPolicy> getEffectivePolicies(Account caller, ControlledEntity entity);
 
     /* Visibility related interfaces */
+    boolean isGrantedAll(long accountId, String action);
+
     List<Long> getGrantedDomains(long accountId, String action);
 
     List<Long> getGrantedAccounts(long accountId, String action);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/630b7fb4/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 29f3ee6..b807f4a 100644
--- a/server/src/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/com/cloud/api/query/QueryManagerImpl.java
@@ -2798,29 +2798,38 @@ public class QueryManagerImpl extends ManagerBase implements QueryService {
             listAll = true;
         }
 
-        List<Long> permittedAccountIds = new ArrayList<Long>();
+        List<Long> permittedDomains = new ArrayList<Long>();
+        List<Long> permittedAccounts = new ArrayList<Long>();
+        List<Long> permittedResources = new ArrayList<Long>();
+
+        //List<Long> permittedAccountIds = new ArrayList<Long>();
         Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(
                 cmd.getDomainId(), cmd.isRecursive(), null);
-        _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedAccountIds,
-                domainIdRecursiveListProject, listAll, false);
+        _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedDomains, permittedAccounts, permittedResources,
+                domainIdRecursiveListProject, listAll, false, "listTemplates");
+
+        //_accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedAccountIds,
+        //        domainIdRecursiveListProject, listAll, false);
+        Boolean isRecursive = domainIdRecursiveListProject.second();
         ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
-        List<Account> permittedAccounts = new ArrayList<Account>();
-        for (Long accountId : permittedAccountIds) {
-            permittedAccounts.add(_accountMgr.getAccount(accountId));
-        }
+        //List<Account> permittedAccounts = new ArrayList<Account>();
+        //for (Long accountId : permittedAccountIds) {
+        //    permittedAccounts.add(_accountMgr.getAccount(accountId));
+        //}
 
         boolean showDomr = ((templateFilter != TemplateFilter.selfexecutable) && (templateFilter != TemplateFilter.featured));
         HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor());
 
         return searchForTemplatesInternal(id, cmd.getTemplateName(), cmd.getKeyword(), templateFilter, false, null,
                 cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType, showDomr,
-                cmd.listInReadyState(), permittedAccounts, caller, listProjectResourcesCriteria, tags);
+                cmd.listInReadyState(), permittedDomains, permittedAccounts, permittedResources, isRecursive, caller, listProjectResourcesCriteria, tags);
     }
 
     private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long templateId, String name,
             String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Long pageSize,
             Long startIndex, Long zoneId, HypervisorType hyperType, boolean showDomr, boolean onlyReady,
-            List<Account> permittedAccounts, Account caller, ListProjectResourcesCriteria listProjectResourcesCriteria,
+            List<Long> permittedDomains, List<Long> permittedAccounts, List<Long> permittedResources, boolean isRecursive, Account caller,
+            ListProjectResourcesCriteria listProjectResourcesCriteria,
             Map<String, String> tags) {
 
         // check if zone is configured, if not, just return empty list
@@ -2873,59 +2882,6 @@ public class QueryManagerImpl extends ManagerBase implements QueryService {
             // search and ignore other query parameters
             sc.addAnd("id", SearchCriteria.Op.EQ, templateId);
         } else {
-
-            DomainVO domain = null;
-            if (!permittedAccounts.isEmpty()) {
-                domain = _domainDao.findById(permittedAccounts.get(0).getDomainId());
-            } else {
-                domain = _domainDao.findById(DomainVO.ROOT_DOMAIN);
-            }
-
-            // List<HypervisorType> hypers = null;
-            // if (!isIso) {
-            // hypers = _resourceMgr.listAvailHypervisorInZone(null, null);
-            // }
-
-            // add criteria for project or not
-            if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
-                sc.addAnd("accountType", SearchCriteria.Op.NEQ, Account.ACCOUNT_TYPE_PROJECT);
-            } else if (listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) {
-                sc.addAnd("accountType", SearchCriteria.Op.EQ, Account.ACCOUNT_TYPE_PROJECT);
-            }
-
-            // add criteria for domain path in case of domain admin
-            if ((templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable)
-                    && (_accountMgr.isDomainAdmin(caller.getId()) || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN)) {
-                sc.addAnd("domainPath", SearchCriteria.Op.LIKE, domain.getPath() + "%");
-            }
-
-            List<Long> relatedDomainIds = new ArrayList<Long>();
-            List<Long> permittedAccountIds = new ArrayList<Long>();
-            if (!permittedAccounts.isEmpty()) {
-                for (Account account : permittedAccounts) {
-                    permittedAccountIds.add(account.getId());
-                    DomainVO accountDomain = _domainDao.findById(account.getDomainId());
-
-                    // get all parent domain ID's all the way till root domain
-                    DomainVO domainTreeNode = accountDomain;
-                    relatedDomainIds.add(domainTreeNode.getId());
-                    while (domainTreeNode.getParent() != null) {
-                        domainTreeNode = _domainDao.findById(domainTreeNode.getParent());
-                        relatedDomainIds.add(domainTreeNode.getId());
-                    }
-
-                    // get all child domain ID's
-                    if (_accountMgr.isAdmin(account.getType())
-                            || (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community)) {
-                        List<DomainVO> allChildDomains = _domainDao.findAllChildren(accountDomain.getPath(),
-                                accountDomain.getId());
-                        for (DomainVO childDomain : allChildDomains) {
-                            relatedDomainIds.add(childDomain.getId());
-                        }
-                    }
-                }
-            }
-
             if (!isIso) {
                 // add hypervisor criteria for template case
                 if (hypers != null && !hypers.isEmpty()) {
@@ -2938,6 +2894,7 @@ public class QueryManagerImpl extends ManagerBase implements QueryService {
             }
 
             // control different template filters
+            DomainVO callerDomain = _domainDao.findById(caller.getDomainId());
             if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) {
                 sc.addAnd("publicTemplate", SearchCriteria.Op.EQ, true);
                 if (templateFilter == TemplateFilter.featured) {
@@ -2945,26 +2902,55 @@ public class QueryManagerImpl extends ManagerBase implements QueryService {
                 } else {
                     sc.addAnd("featured", SearchCriteria.Op.EQ, false);
                 }
-                if (!permittedAccounts.isEmpty()) {
-                    SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
-                    scc.addOr("domainId", SearchCriteria.Op.IN, relatedDomainIds.toArray());
-                    scc.addOr("domainId", SearchCriteria.Op.NULL);
-                    sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
+
+                // restrict to caller domain tree
+                // get all parent domain ID's all the way till root domain
+                List<Long> domainTree = new ArrayList<Long>();
+                DomainVO domainTreeNode = callerDomain;
+                domainTree.add(domainTreeNode.getId());
+                while (domainTreeNode.getParent() != null) {
+                    domainTreeNode = _domainDao.findById(domainTreeNode.getParent());
+                    domainTree.add(domainTreeNode.getId());
+                }
+
+                // get all child domain ID's
+                List<DomainVO> allChildDomains = _domainDao.findAllChildren(callerDomain.getPath(),
+                        callerDomain.getId());
+                for (DomainVO childDomain : allChildDomains) {
+                    domainTree.add(childDomain.getId());
                 }
+
+                SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
+                scc.addOr("domainId", SearchCriteria.Op.IN, domainTree.toArray());
+                scc.addOr("domainId", SearchCriteria.Op.NULL);
+                sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
             } else if (templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable) {
-                if (!permittedAccounts.isEmpty()) {
-                    sc.addAnd("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
+                if (permittedDomains.contains(caller.getDomainId())) {
+                    // this caller acts like a domain admin
+
+                    sc.addAnd("domainPath", SearchCriteria.Op.LIKE, callerDomain.getPath() + "%");
+                } else {
+                    // only display templates owned by caller for resource owner only
+                    sc.addAnd("accountId", SearchCriteria.Op.EQ, caller.getAccountId());
                 }
             } else if (templateFilter == TemplateFilter.sharedexecutable || templateFilter == TemplateFilter.shared) {
-                SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
-                scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
-                scc.addOr("sharedAccountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
-                sc.addAnd("accountId", SearchCriteria.Op.SC, scc);
+                // exclude the caller, only include those granted and not owned by self
+                permittedDomains.remove(caller.getDomainId());
+                permittedAccounts.remove(caller.getAccountId());
+                // building ACL search criteria
+                SearchCriteria<TemplateJoinVO> aclSc = _templateJoinDao.createSearchCriteria();
+                _accountMgr.buildACLViewSearchCriteria(sc, aclSc, isRecursive, permittedDomains, permittedAccounts, permittedResources, listProjectResourcesCriteria);
             } else if (templateFilter == TemplateFilter.executable) {
+                // public template + self template
                 SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
                 scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
-                if (!permittedAccounts.isEmpty()) {
-                    scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
+                // plus self owned templates or domain tree templates for domain admin
+                if (permittedDomains.contains(caller.getDomainId())) {
+                    // this caller acts like a domain admin
+                    sc.addOr("domainPath", SearchCriteria.Op.LIKE, callerDomain.getPath() + "%");
+                } else {
+                    // only display templates owned by caller for resource owner only
+                    sc.addOr("accountId", SearchCriteria.Op.EQ, caller.getAccountId());
                 }
                 sc.addAnd("publicTemplate", SearchCriteria.Op.SC, scc);
             }
@@ -3100,22 +3086,26 @@ public class QueryManagerImpl extends ManagerBase implements QueryService {
             listAll = true;
         }
 
-        List<Long> permittedAccountIds = new ArrayList<Long>();
+        List<Long> permittedDomains = new ArrayList<Long>();
+        List<Long> permittedAccounts = new ArrayList<Long>();
+        List<Long> permittedResources = new ArrayList<Long>();
+
         Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(
                 cmd.getDomainId(), cmd.isRecursive(), null);
-        _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedAccountIds,
-                domainIdRecursiveListProject, listAll, false);
+        _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedDomains, permittedAccounts, permittedResources,
+                domainIdRecursiveListProject, cmd.listAll(), false, "listIsos");
+        Boolean isRecursive = domainIdRecursiveListProject.second();
         ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
-        List<Account> permittedAccounts = new ArrayList<Account>();
-        for (Long accountId : permittedAccountIds) {
-            permittedAccounts.add(_accountMgr.getAccount(accountId));
-        }
+//        List<Account> permittedAccounts = new ArrayList<Account>();
+//        for (Long accountId : permittedAccountIds) {
+//            permittedAccounts.add(_accountMgr.getAccount(accountId));
+//        }
 
         HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor());
 
         return searchForTemplatesInternal(cmd.getId(), cmd.getIsoName(), cmd.getKeyword(), isoFilter, true,
                 cmd.isBootable(), cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType, true,
-                cmd.listInReadyState(), permittedAccounts, caller, listProjectResourcesCriteria, tags);
+                cmd.listInReadyState(), permittedDomains, permittedAccounts, permittedResources, isRecursive, caller, listProjectResourcesCriteria, tags);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/630b7fb4/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 f3f73e7..adabec6 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -2422,6 +2422,7 @@ 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
@@ -2489,28 +2490,36 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             // search for policy permissions associated with caller to get all his authorized domains, accounts, and resources
             // Assumption: if a domain is in grantedDomains, then all the accounts under this domain will not be returned in "grantedAccounts". Similarly, if an account
             // is in grantedAccounts, then all the resources owned by this account will not be returned in "grantedResources".
-            List<Long> grantedDomains = _aclService.getGrantedDomains(caller.getId(), action);
-            List<Long> grantedAccounts = _aclService.getGrantedAccounts(caller.getId(), action);
-            List<Long> grantedResources = _aclService.getGrantedResources(caller.getId(), action);
-
-            if (domainId != null) {
-                // specific domain is specified
-                if (grantedDomains.contains(domainId)) {
+            boolean grantedAll = _aclService.isGrantedAll(caller.getId(), action);
+            if ( grantedAll ){
+                if ( domainId != null ){
                     permittedDomains.add(domainId);
-                } else {
-                    for (Long acctId : grantedAccounts) {
-                        Account acct = _accountDao.findById(acctId);
-                        if (acct != null && acct.getDomainId() == domainId) {
-                            permittedAccounts.add(acctId);
+                }
+            }
+            else {
+                List<Long> grantedDomains = _aclService.getGrantedDomains(caller.getId(), action);
+                List<Long> grantedAccounts = _aclService.getGrantedAccounts(caller.getId(), action);
+                List<Long> grantedResources = _aclService.getGrantedResources(caller.getId(), action);
+
+                if (domainId != null) {
+                    // specific domain is specified
+                    if (grantedDomains.contains(domainId)) {
+                        permittedDomains.add(domainId);
+                    } else {
+                        for (Long acctId : grantedAccounts) {
+                            Account acct = _accountDao.findById(acctId);
+                            if (acct != null && acct.getDomainId() == domainId) {
+                                permittedAccounts.add(acctId);
+                            }
                         }
+                        permittedResources.addAll(grantedResources);
                     }
+                } else if (permittedAccounts.isEmpty()) {
+                    // neither domain nor account is not specified
+                    permittedDomains.addAll(grantedDomains);
+                    permittedAccounts.addAll(grantedAccounts);
                     permittedResources.addAll(grantedResources);
                 }
-            } else if (permittedAccounts.isEmpty()) {
-                // neither domain nor account is not specified
-                permittedDomains.addAll(grantedDomains);
-                permittedAccounts.addAll(grantedAccounts);
-                permittedResources.addAll(grantedResources);
             }
         }
 
@@ -2523,9 +2532,18 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             List<Long> permittedAccounts, List<Long> permittedResources, ListProjectResourcesCriteria listProjectResourcesCriteria) {
 
         if (listProjectResourcesCriteria != null) {
-            sc.addAnd("accountType", SearchCriteria.Op.EQ, Account.ACCOUNT_TYPE_PROJECT);
+            // add criteria for project or not
+            if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
+                sc.addAnd("accountType", SearchCriteria.Op.NEQ, Account.ACCOUNT_TYPE_PROJECT);
+            } else if (listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) {
+                sc.addAnd("accountType", SearchCriteria.Op.EQ, Account.ACCOUNT_TYPE_PROJECT);
+            }
         }
 
+        if (permittedDomains.isEmpty() && permittedAccounts.isEmpty() && permittedResources.isEmpty())
+            // can access everything
+            return;
+
         // Note that this may have limitations on number of permitted domains, accounts, or resource ids are allowed due to sql package size limitation
         if (!permittedDomains.isEmpty()) {
             if (isRecursive) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/630b7fb4/server/src/org/apache/cloudstack/acl/AclServiceImpl.java
----------------------------------------------------------------------
diff --git a/server/src/org/apache/cloudstack/acl/AclServiceImpl.java b/server/src/org/apache/cloudstack/acl/AclServiceImpl.java
index 3581ef0..8ed6351 100644
--- a/server/src/org/apache/cloudstack/acl/AclServiceImpl.java
+++ b/server/src/org/apache/cloudstack/acl/AclServiceImpl.java
@@ -27,7 +27,6 @@ import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.AclPolicyPermission.Permission;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
-import org.apache.cloudstack.acl.dao.AclApiPermissionDao;
 import org.apache.cloudstack.acl.dao.AclGroupAccountMapDao;
 import org.apache.cloudstack.acl.dao.AclGroupDao;
 import org.apache.cloudstack.acl.dao.AclGroupPolicyMapDao;
@@ -733,4 +732,19 @@ public class AclServiceImpl extends ManagerBase implements AclService, Manager {
         return entityIds;
     }
 
+    @Override
+    public boolean isGrantedAll(long accountId, String action) {
+        // Get the static Policies of the Caller
+        List<AclPolicy> policies = listAclPolicies(accountId);
+        // for each policy, find granted permission with Domain scope
+        List<Long> domainIds = new ArrayList<Long>();
+        for (AclPolicy policy : policies) {
+            List<AclPolicyPermissionVO> pp = _policyPermissionDao.listGrantedByActionAndScope(policy.getId(), action, PermissionScope.ALL);
+            if (pp != null && pp.size() > 0) {
+                return true;
+            }
+        }
+        return false;
+    }
+
 }