Skip to content

Commit debfb45

Browse files
soreanaweizhouapache
soreana
and
weizhouapache
authored
Added configuration and Integration test to restrict public template … (#4774)
* Added configuration and Integration test to restrict public template access. * Move settings to domain. * Updated integration test. * Changed Config key's name and description. * Justified the variable names and removed white spaces. * Added configuration and Integration test to restrict public template access. * Move settings to domain. * Changed Config key's name and description. * Justified the variable names and removed white spaces. * Moved configuration to domain scope. * Added integration test to travis. * Updated the configuration's name and description. * Extracted public template check to a separate method. * Fixed rebase issue. * Apply tear down changes. * Update .travis.yml to remove the component test The test needs to be updated to use the new configuration name Co-authored-by: Wei Zhou <[email protected]>
1 parent 830f306 commit debfb45

File tree

4 files changed

+693
-5
lines changed

4 files changed

+693
-5
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ public interface QueryService {
111111
"allow.user.view.all.domain.accounts", "false",
112112
"Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
113113

114+
static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
115+
"If false, templates of this domain will not show up in the list templates of other domains.", true, ConfigKey.Scope.Domain);
116+
114117
ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;
115118

116119
ListResponse<UserResponse> searchForUsers(Long domainId, boolean recursive) throws PermissionDeniedException;

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.cloudstack.acl.SecurityChecker;
2929
import org.apache.cloudstack.affinity.AffinityGroup;
3030
import org.apache.cloudstack.context.CallContext;
31+
import org.apache.cloudstack.query.QueryService;
3132
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
3233
import org.apache.log4j.Logger;
3334
import org.springframework.stereotype.Component;
@@ -103,6 +104,38 @@ protected DomainChecker() {
103104
super();
104105
}
105106

107+
/**
108+
*
109+
* public template can be used by other accounts in:
110+
*
111+
* 1. the same domain
112+
* 2. in sub-domains
113+
* 3. domain admin of parent domains
114+
*
115+
* In addition to those, everyone can access the public templates in domains that set "share.public.templates.with.other.domains" config to true.
116+
*
117+
* @param template template object
118+
* @param owner owner of the template
119+
* @param caller who wants to access to the template
120+
*/
121+
122+
private void checkPublicTemplateAccess(VirtualMachineTemplate template, Account owner, Account caller){
123+
if (!QueryService.SharePublicTemplatesWithOtherDomains.valueIn(owner.getDomainId()) ||
124+
caller.getDomainId() == owner.getDomainId() ||
125+
_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
126+
return;
127+
}
128+
129+
if (caller.getType() == Account.Type.NORMAL || caller.getType() == Account.Type.PROJECT) {
130+
throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
131+
} else if (caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) {
132+
if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
133+
throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
134+
}
135+
}
136+
}
137+
138+
106139
@Override
107140
public boolean checkAccess(Account caller, Domain domain) throws PermissionDeniedException {
108141
if (caller.getState() != Account.State.ENABLED) {
@@ -168,6 +201,8 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
168201
throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
169202
}
170203
}
204+
} else if (caller.getType() != Account.Type.ADMIN) {
205+
checkPublicTemplateAccess(template, owner, caller);
171206
}
172207
}
173208

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

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.ListIterator;
2727
import java.util.Map;
2828
import java.util.Set;
29+
import java.util.function.Predicate;
2930
import java.util.stream.Collectors;
3031
import java.util.stream.Stream;
3132

@@ -3636,6 +3637,8 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long temp
36363637
// if template is not public, perform permission check here
36373638
else if (!template.isPublicTemplate() && caller.getType() != Account.Type.ADMIN) {
36383639
_accountMgr.checkAccess(caller, null, false, template);
3640+
} else if (template.isPublicTemplate()) {
3641+
_accountMgr.checkAccess(caller, null, false, template);
36393642
}
36403643

36413644
// 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)
37413744
}
37423745
}
37433746

3744-
return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr,
3747+
return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller,
37453748
showRemovedTmpl, parentTemplateId, showUnique, searchFilter, sc);
37463749

37473750
}
37483751

37493752
private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword,
3750-
HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr,
3753+
HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Account caller,
37513754
boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique,
37523755
Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) {
37533756
if (!isIso) {
@@ -3849,7 +3852,7 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
38493852
}
38503853
}
38513854

3852-
return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique);
3855+
return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique, caller);
38533856

38543857
// TODO: revisit the special logic for iso search in
38553858
// VMTemplateDaoImpl.searchForTemplates and understand why we need to
@@ -3858,7 +3861,8 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
38583861
}
38593862

38603863
// findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair
3861-
private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair, boolean showRemoved, boolean showUnique) {
3864+
private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair,
3865+
boolean showRemoved, boolean showUnique, Account caller) {
38623866
Integer count = templateDataPair.second();
38633867
if (count.intValue() == 0) {
38643868
// empty result
@@ -3873,9 +3877,28 @@ private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair
38733877
String[] templateZonePairs = templateData.stream().map(template -> template.getTempZonePair()).toArray(String[]::new);
38743878
templates = _templateJoinDao.searchByTemplateZonePair(showRemoved, templateZonePairs);
38753879
}
3880+
3881+
if(caller.getType() != Account.Type.ADMIN) {
3882+
templates = applyPublicTemplateRestriction(templates, caller);
3883+
count = templates.size();
3884+
}
3885+
38763886
return new Pair<List<TemplateJoinVO>, Integer>(templates, count);
38773887
}
38783888

3889+
private List<TemplateJoinVO> applyPublicTemplateRestriction(List<TemplateJoinVO> templates, Account caller){
3890+
List<Long> unsharableDomainIds = templates.stream()
3891+
.map(TemplateJoinVO::getDomainId)
3892+
.distinct()
3893+
.filter(domainId -> domainId != caller.getDomainId())
3894+
.filter(Predicate.not(QueryService.SharePublicTemplatesWithOtherDomains::valueIn))
3895+
.collect(Collectors.toList());
3896+
3897+
return templates.stream()
3898+
.filter(Predicate.not(t -> unsharableDomainIds.contains(t.getDomainId())))
3899+
.collect(Collectors.toList());
3900+
}
3901+
38793902
@Override
38803903
public ListResponse<TemplateResponse> listIsos(ListIsosCmd cmd) {
38813904
Pair<List<TemplateJoinVO>, Integer> result = searchForIsosInternal(cmd);
@@ -4357,6 +4380,7 @@ public String getConfigComponentName() {
43574380

43584381
@Override
43594382
public ConfigKey<?>[] getConfigKeys() {
4360-
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending, AllowUserViewAllDomainAccounts};
4383+
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending,
4384+
AllowUserViewAllDomainAccounts, SharePublicTemplatesWithOtherDomains};
43614385
}
43624386
}

0 commit comments

Comments
 (0)