Added configuration and Integration test to restrict public template access.

Author: Sina Kashipazha

engine/components-api/src/main/java/com/cloud/template/TemplateManager.java

@@ -42,14 +42,16 @@
 public interface TemplateManager {
     static final String AllowPublicUserTemplatesCK = "allow.public.user.templates";
     static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size";
+    static final String RestrictPublicTemplateAccessToDomainCK = "restrict.public.template.access.to.domain";
 
     static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
         "If false, users will not be able to create public templates.", true, ConfigKey.Scope.Account);
 
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
-
+    static final ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, RestrictPublicTemplateAccessToDomainCK, "false",
+            "If true, the public template wouldn't share between domains", true, ConfigKey.Scope.Global);
 
     /**
      * Prepares a template for vm creation for a certain storage pool.

server/src/main/java/com/cloud/acl/DomainChecker.java

@@ -55,6 +55,7 @@
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.LaunchPermissionVO;
 import com.cloud.storage.dao.LaunchPermissionDao;
+import com.cloud.template.TemplateManager;
 import com.cloud.template.VirtualMachineTemplate;
 import com.cloud.user.Account;
 import com.cloud.user.AccountService;
@@ -166,6 +167,16 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
                             throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
                         }
                     }
+                } else if (TemplateManager.RestrictPublicTemplateAccessToDomain.value() && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { // public template can be used by other accounts in the same domain or in sub-domains, and domain admin of parent domains
+                    if (caller.getDomainId() != owner.getDomainId() && !_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
+                        if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL || caller.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                            throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                        } else if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) {
+                            if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+                                throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                            }
+                        }
+                    }
                 }
             }
 

server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

@@ -218,6 +218,7 @@
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.tags.ResourceTagVO;
 import com.cloud.tags.dao.ResourceTagDao;
+import com.cloud.template.TemplateManager;
 import com.cloud.template.VirtualMachineTemplate.State;
 import com.cloud.template.VirtualMachineTemplate.TemplateFilter;
 import com.cloud.user.Account;
@@ -3355,7 +3356,7 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(ListTempl
         Long parentTemplateId = cmd.getParentTemplateId();
 
         boolean listAll = false;
-        if (templateFilter != null && templateFilter == TemplateFilter.all) {
+        if (TemplateFilter.all == templateFilter) {
             if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL) {
                 throw new InvalidParameterValueException("Filter " + TemplateFilter.all + " can be specified by admin only");
             }
@@ -3365,21 +3366,48 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(ListTempl
         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);
+        Long domainId = domainIdRecursiveListProject.first();
         ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
         List<Account> permittedAccounts = new ArrayList<Account>();
         for (Long accountId : permittedAccountIds) {
             permittedAccounts.add(_accountMgr.getAccount(accountId));
         }
 
-        boolean showDomr = ((templateFilter != TemplateFilter.selfexecutable) && (templateFilter != TemplateFilter.featured));
+        boolean showDomr = ((templateFilter != TemplateFilter.selfexecutable) && (templateFilter != TemplateFilter.featured) && _accountMgr.isRootAdmin(caller.getId()));
         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, showRemovedTmpl, cmd.getIds(), parentTemplateId, cmd.getShowUnique());
+                showDomr, cmd.listInReadyState(), permittedAccounts, caller, domainId, listProjectResourcesCriteria, tags, showRemovedTmpl, cmd.getIds(), parentTemplateId, cmd.getShowUnique());
+    }
+
+    private DomainVO getDomainOf(TemplateFilter templateFilter, Account account ) {
+        boolean publicTemplates = (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community || templateFilter == TemplateFilter.executable);
+
+        // get all parent domain ID's all the way till root domain
+        //if template filter is featured, or community, all child domains should be included in search
+        if (publicTemplates)
+            return _domainDao.findById(Domain.ROOT_DOMAIN);
+        else
+            return _domainDao.findById(account.getDomainId());
+    }
+
+    private ArrayList<Long> getChildDomainIds(boolean isAdmin, TemplateFilter templateFilter, DomainVO domainTreeNode ){
+        boolean publicTemplates = (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community || templateFilter == TemplateFilter.executable);
+        ArrayList<Long> domainIds = new ArrayList<>();
+
+        // get all child domain ID's
+        if (isAdmin || publicTemplates) {
+            List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
+            for (DomainVO childDomain : allChildDomains) {
+                domainIds.add(childDomain.getId());
+            }
+        }
+
+        return domainIds;
     }
 
     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,
+            Long startIndex, Long zoneId, HypervisorType hyperType, boolean showDomr, boolean onlyReady, List<Account> permittedAccounts, Account caller, Long domainId,
             ListProjectResourcesCriteria listProjectResourcesCriteria, Map<String, String> tags, boolean showRemovedTmpl, List<Long> ids, Long parentTemplateId, Boolean showUnique) {
 
         // check if zone is configured, if not, just return empty list
@@ -3407,6 +3435,8 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long temp
         }
         SearchCriteria<TemplateJoinVO> sc = sb.create();
 
+        boolean restrictPublicTemplatesToDomain = TemplateManager.RestrictPublicTemplateAccessToDomain.value();
+
         // verify templateId parameter and specially handle it
         if (templateId != null) {
             template = _templateDao.findByIdIncludingRemoved(templateId); // Done for backward compatibility - Bug-5221
@@ -3434,6 +3464,8 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long temp
             // if template is not public, perform permission check here
             else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
                 _accountMgr.checkAccess(caller, null, false, template);
+            } else if (template.isPublicTemplate()) {
+                _accountMgr.checkAccess(caller, null, false, template);
             }
 
             // if templateId is specified, then we will just use the id to
@@ -3444,6 +3476,8 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
             DomainVO domain = null;
             if (!permittedAccounts.isEmpty()) {
                 domain = _domainDao.findById(permittedAccounts.get(0).getDomainId());
+            } else if (restrictPublicTemplatesToDomain && domainId != null) {
+                domain = _domainDao.findById(domainId);
             } else {
                 domain = _domainDao.findById(Domain.ROOT_DOMAIN);
             }
@@ -3468,31 +3502,34 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
             if (!permittedAccounts.isEmpty()) {
                 for (Account account : permittedAccounts) {
                     permittedAccountIds.add(account.getId());
-                    boolean publicTemplates = (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community);
 
-                    // get all parent domain ID's all the way till root domain
-                    DomainVO domainTreeNode = null;
-                    //if template filter is featured, or community, all child domains should be included in search
-                    if (publicTemplates) {
-                        domainTreeNode = _domainDao.findById(Domain.ROOT_DOMAIN);
+                    DomainVO domainTreeNode = domain;
 
-                    } else {
-                        domainTreeNode = _domainDao.findById(account.getDomainId());
+                    if(! restrictPublicTemplatesToDomain) {
+                        domainTreeNode = getDomainOf(templateFilter, account);
                     }
                     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.getId()) || publicTemplates) {
-                        List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
-                        for (DomainVO childDomain : allChildDomains) {
-                            relatedDomainIds.add(childDomain.getId());
-                        }
+                    boolean isAdmin = _accountMgr.isAdmin(account.getId());
+                    if(! restrictPublicTemplatesToDomain) {
+                        relatedDomainIds.addAll(getChildDomainIds(isAdmin, templateFilter, domainTreeNode));
+                    } else if (isAdmin) {
+                        relatedDomainIds.addAll(getChildDomainIds(isAdmin, templateFilter, domain));
                     }
                 }
+            } else if (restrictPublicTemplatesToDomain && domainId != null) {
+                // templatefilter=all or domainid is passed, called by root admin/domain admin
+                DomainVO domainTreeNode = domain;
+                relatedDomainIds.add(domainTreeNode.getId());
+                while (domainTreeNode.getParent() != null) {
+                    domainTreeNode = _domainDao.findById(domainTreeNode.getParent());
+                    relatedDomainIds.add(domainTreeNode.getId());
+                }
+                relatedDomainIds.addAll(getChildDomainIds(true, templateFilter, domain));
             }
 
             // control different template filters
@@ -3517,18 +3554,38 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
                 // only show templates shared by others
                 sc.addAnd("sharedAccountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
             } else if (templateFilter == TemplateFilter.executable) {
+                SearchCriteria<TemplateJoinVO> scc2 = _templateJoinDao.createSearchCriteria();
+                scc2.addAnd("publicTemplate", SearchCriteria.Op.EQ, true);
+                if (restrictPublicTemplatesToDomain) {
+                    SearchCriteria<TemplateJoinVO> scc3 = _templateJoinDao.createSearchCriteria();
+                    scc3.addOr("domainId", SearchCriteria.Op.IN, relatedDomainIds.toArray());
+                    scc3.addOr("domainId", SearchCriteria.Op.NULL);
+                    scc2.addAnd("domainId", SearchCriteria.Op.SC, scc3);
+                }
                 SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
-                scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
+                scc.addOr("publicTemplate", SearchCriteria.Op.SC, scc2);
                 if (!permittedAccounts.isEmpty()) {
                     scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
                 }
                 sc.addAnd("publicTemplate", SearchCriteria.Op.SC, scc);
-            } else if (templateFilter == TemplateFilter.all && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
+            } else if (templateFilter == TemplateFilter.all && (caller.getType() != Account.ACCOUNT_TYPE_ADMIN || domainId != null)) {
+                SearchCriteria<TemplateJoinVO> scc2 = _templateJoinDao.createSearchCriteria();
+                scc2.addAnd("publicTemplate", SearchCriteria.Op.EQ, true);
+                if (restrictPublicTemplatesToDomain) {
+                    SearchCriteria<TemplateJoinVO> scc3 = _templateJoinDao.createSearchCriteria();
+                    scc3.addOr("domainId", SearchCriteria.Op.IN, relatedDomainIds.toArray());
+                    scc3.addOr("domainId", SearchCriteria.Op.NULL);
+                    scc2.addAnd("domainId", SearchCriteria.Op.SC, scc3);
+                }
                 SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
-                scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
+                scc.addOr("publicTemplate", SearchCriteria.Op.SC, scc2);
 
                 if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
-                    scc.addOr("domainPath", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
+                    if (domainId != null) {
+                        scc.addOr("domainPath", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
+                    } else {
+                        scc.addOr("domainPath", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
+                    }
                 } else {
                     if (!permittedAccounts.isEmpty()) {
                         scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
@@ -3539,13 +3596,13 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
             }
         }
 
-        return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr,
+        return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, domainId,
                 showRemovedTmpl, parentTemplateId, showUnique, searchFilter, sc);
 
     }
 
     private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword,
-                                                               HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr,
+                                                               HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Long domainId,
                                                                boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique,
                                                                Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) {
         if (!isIso) {
@@ -3606,7 +3663,7 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
             sc.addAnd("state", SearchCriteria.Op.SC, readySc);
         }
 
-        if (!showDomr) {
+        if (!showDomr || domainId != null) {
             // excluding system template
             sc.addAnd("templateType", SearchCriteria.Op.NEQ, Storage.TemplateType.SYSTEM);
         }
@@ -3707,6 +3764,7 @@ private Pair<List<TemplateJoinVO>, Integer> searchForIsosInternal(ListIsosCmd cm
         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);
+        Long domainId = domainIdRecursiveListProject.first();
         ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
         List<Account> permittedAccounts = new ArrayList<Account>();
         for (Long accountId : permittedAccountIds) {
@@ -3716,7 +3774,7 @@ private Pair<List<TemplateJoinVO>, Integer> searchForIsosInternal(ListIsosCmd cm
         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, showRemovedISO, null, null, cmd.getShowUnique());
+                hypervisorType, true, cmd.listInReadyState(), permittedAccounts, caller, domainId, listProjectResourcesCriteria, tags, showRemovedISO, null, null, cmd.getShowUnique());
     }
 
     @Override

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

@@ -2243,7 +2243,7 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {AllowPublicUserTemplates, TemplatePreloaderPoolSize};
+        return new ConfigKey<?>[] {AllowPublicUserTemplates, TemplatePreloaderPoolSize, RestrictPublicTemplateAccessToDomain};
     }
 
     public List<TemplateAdapter> getTemplateAdapters() {