Added configuration and Integration test to restrict public template …

api/src/main/java/org/apache/cloudstack/query/QueryService.java

@@ -111,6 +111,9 @@ public interface QueryService {
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
+    static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
+            "If false, templates of this domain will not show up in the list templates of other domains.", true, ConfigKey.Scope.Domain);
+
     ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;
 
     ListResponse<UserResponse> searchForUsers(Long domainId, boolean recursive) throws PermissionDeniedException;

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

@@ -28,6 +28,7 @@
 import org.apache.cloudstack.acl.SecurityChecker;
 import org.apache.cloudstack.affinity.AffinityGroup;
 import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.query.QueryService;
 import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -102,6 +103,38 @@ protected DomainChecker() {
         super();
     }
 
+    /**
+     *
+     * public template can be used by other accounts in:
+     *
+     *  1. the same domain
+     *  2. in sub-domains
+     *  3. domain admin of parent domains
+     *
+     *  In addition to those, everyone can access the public templates in domains that set "share.public.templates.with.other.domains" config to true.
+     *
+     * @param template template object
+     * @param owner owner of the template
+     * @param caller who wants to access to the template
+     */
+
+    private void checkPublicTemplateAccess(VirtualMachineTemplate template, Account owner, Account caller){
+        if (!QueryService.SharePublicTemplatesWithOtherDomains.valueIn(owner.getDomainId()) ||
+                caller.getDomainId() == owner.getDomainId() ||
+                _domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
+            return;
+        }
+
+        if (caller.getType() == Account.Type.NORMAL || caller.getType() == Account.Type.PROJECT) {
+            throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+        } else if (caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) {
+            if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+                throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+            }
+        }
+    }
+
+
     @Override
     public boolean checkAccess(Account caller, Domain domain) throws PermissionDeniedException {
         if (caller.getState() != Account.State.ENABLED) {
@@ -167,6 +200,8 @@ 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 (caller.getType() != Account.Type.ADMIN) {
+                    checkPublicTemplateAccess(template, owner, caller);
                 }
             }
 

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

@@ -26,6 +26,7 @@
 import java.util.ListIterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -3636,6 +3637,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.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
@@ -3741,13 +3744,13 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.Type.ADMIN)
             }
         }
 
-        return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr,
+        return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller,
                 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, Account caller,
                                                                boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique,
                                                                Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) {
         if (!isIso) {
@@ -3849,7 +3852,7 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
             }
         }
 
-        return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique);
+        return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique, caller);
 
         // TODO: revisit the special logic for iso search in
         // VMTemplateDaoImpl.searchForTemplates and understand why we need to
@@ -3858,7 +3861,8 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
     }
 
     // findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair
-    private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair, boolean showRemoved, boolean showUnique) {
+    private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair,
+                                                                                boolean showRemoved, boolean showUnique, Account caller) {
         Integer count = templateDataPair.second();
         if (count.intValue() == 0) {
             // empty result
@@ -3873,9 +3877,28 @@ private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair
             String[] templateZonePairs = templateData.stream().map(template -> template.getTempZonePair()).toArray(String[]::new);
             templates = _templateJoinDao.searchByTemplateZonePair(showRemoved, templateZonePairs);
         }
+
+        if(caller.getType() != Account.Type.ADMIN) {
+            templates = applyPublicTemplateRestriction(templates, caller);
+            count = templates.size();
+        }
+
         return new Pair<List<TemplateJoinVO>, Integer>(templates, count);
     }
 
+    private List<TemplateJoinVO> applyPublicTemplateRestriction(List<TemplateJoinVO> templates, Account caller){
+        List<Long> unsharableDomainIds = templates.stream()
+                .map(TemplateJoinVO::getDomainId)
+                .distinct()
+                .filter(domainId -> domainId != caller.getDomainId())
+                .filter(Predicate.not(QueryService.SharePublicTemplatesWithOtherDomains::valueIn))
+                .collect(Collectors.toList());
+
+        return templates.stream()
+                .filter(Predicate.not(t -> unsharableDomainIds.contains(t.getDomainId())))
+                .collect(Collectors.toList());
+    }
+
     @Override
     public ListResponse<TemplateResponse> listIsos(ListIsosCmd cmd) {
         Pair<List<TemplateJoinVO>, Integer> result = searchForIsosInternal(cmd);
@@ -4357,6 +4380,7 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending, AllowUserViewAllDomainAccounts};
+        return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending,
+                AllowUserViewAllDomainAccounts, SharePublicTemplatesWithOtherDomains};
     }
 }