Skip to content

Commit 6e8a015

Browse files
nlgordonnlgordon
authored
AC-110 restrict public template access (#46)
* Pulling in the relevant parts of the global settings version of this PR: apache#4774 in commit e94c1e2. Had to make a few changes to support 4.11 and cleanup some of the quirks. * Making the 'allow.public.user.templates' global setting allow domain and resource admins to still upload public templates. Previously it was limited to just root admins. * One additional template filter type that includes public templates * Fixing permissions bug where there weren't permissions to templates in child domains * Updating comments based on code review * Was previously allowing public, but not featured for domain admins. Added featured for domain admins. Also limited public to only when the new setting is enabled.
1 parent 8300b6f commit 6e8a015

File tree

6 files changed

+77
-22
lines changed

6 files changed

+77
-22
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ public interface QueryService {
8686
static final ConfigKey<Boolean> AllowUserViewDestroyedVM = new ConfigKey<Boolean>("Advanced", Boolean.class, "allow.user.view.destroyed.vm", "false",
8787
"Determines whether users can view their destroyed or expunging vm ", true, ConfigKey.Scope.Account);
8888

89+
ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.template.access.to.domain", "false",
90+
"Prevents a domain from seeing the public templates of another sibling domain. Does not prevent seeing parent or child templates", true, ConfigKey.Scope.Global);
91+
8992
ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;
9093

9194
ListResponse<EventResponse> searchForEvents(ListEventsCmd cmd);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import javax.inject.Inject;
2020

21+
import org.apache.cloudstack.query.QueryService;
2122
import org.springframework.stereotype.Component;
2223

2324
import org.apache.cloudstack.acl.ControlledEntity;
@@ -128,6 +129,11 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
128129
throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
129130
}
130131
}
132+
} else if (QueryService.RestrictPublicTemplateAccessToDomain.value() && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
133+
// Look both up the tree and down from the caller's position
134+
if (!_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId()) && !_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
135+
throw new PermissionDeniedException(caller + " is not allowed to access template " + template);
136+
}
131137
}
132138
}
133139

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

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3063,7 +3063,7 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(ListTempl
30633063
accountId = userAccount != null ? userAccount.getAccountId() : null;
30643064
}
30653065

3066-
boolean showDomr = ((templateFilter != TemplateFilter.selfexecutable) && (templateFilter != TemplateFilter.featured));
3066+
boolean showDomr = ((templateFilter != TemplateFilter.selfexecutable) && (templateFilter != TemplateFilter.featured) && _accountMgr.isRootAdmin(caller.getId()));
30673067
HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor());
30683068

30693069
return searchForTemplatesInternal(id, cmd.getTemplateName(), cmd.getKeyword(), templateFilter, false, null, cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType,
@@ -3130,6 +3130,8 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long temp
31303130
// if template is not public, perform permission check here
31313131
else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
31323132
_accountMgr.checkAccess(caller, null, false, template);
3133+
} else if (template.isPublicTemplate() && RestrictPublicTemplateAccessToDomain.value()) {
3134+
_accountMgr.checkAccess(caller, null, false, template);
31333135
}
31343136

31353137
// if templateId is specified, then we will just use the id to
@@ -3164,36 +3166,58 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
31643166
sc.addAnd("domainPath", SearchCriteria.Op.LIKE, domain.getPath() + "%");
31653167
}
31663168

3169+
boolean publicTemplates = (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community || templateFilter == TemplateFilter.all || templateFilter == TemplateFilter.executable);
31673170
List<Long> relatedDomainIds = new ArrayList<Long>();
31683171
List<Long> permittedAccountIds = new ArrayList<Long>();
31693172
if (!permittedAccounts.isEmpty()) {
31703173
for (Account account : permittedAccounts) {
31713174
permittedAccountIds.add(account.getId());
3172-
boolean publicTemplates = (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community);
31733175

31743176
// get all parent domain ID's all the way till root domain
31753177
DomainVO domainTreeNode = null;
31763178
//if template filter is featured, or community, all child domains should be included in search
3177-
if (publicTemplates) {
3179+
if (publicTemplates && !RestrictPublicTemplateAccessToDomain.value()) {
31783180
domainTreeNode = _domainDao.findById(Domain.ROOT_DOMAIN);
31793181

31803182
} else {
31813183
domainTreeNode = _domainDao.findById(account.getDomainId());
31823184
}
31833185
relatedDomainIds.add(domainTreeNode.getId());
3186+
3187+
// Only get children of the account domain if we are restricting
3188+
if (RestrictPublicTemplateAccessToDomain.value()) {
3189+
if (_accountMgr.isAdmin(account.getId()) || publicTemplates) {
3190+
List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
3191+
for (DomainVO childDomain : allChildDomains) {
3192+
relatedDomainIds.add(childDomain.getId());
3193+
}
3194+
}
3195+
}
31843196
while (domainTreeNode.getParent() != null) {
31853197
domainTreeNode = _domainDao.findById(domainTreeNode.getParent());
31863198
relatedDomainIds.add(domainTreeNode.getId());
31873199
}
3188-
3189-
// get all child domain ID's
3190-
if (_accountMgr.isAdmin(account.getId()) || publicTemplates) {
3191-
List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
3192-
for (DomainVO childDomain : allChildDomains) {
3193-
relatedDomainIds.add(childDomain.getId());
3200+
if (!RestrictPublicTemplateAccessToDomain.value()) {
3201+
// This more or less gets all domains in the system since it walks up the tree first
3202+
if (_accountMgr.isAdmin(account.getId()) || publicTemplates) {
3203+
List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
3204+
for (DomainVO childDomain : allChildDomains) {
3205+
relatedDomainIds.add(childDomain.getId());
3206+
}
31943207
}
31953208
}
31963209
}
3210+
} else if (RestrictPublicTemplateAccessToDomain.value() && publicTemplates && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
3211+
DomainVO domainTreeNode = _domainDao.findById(caller.getDomainId());
3212+
relatedDomainIds.add(domainTreeNode.getId());
3213+
List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
3214+
for (DomainVO childDomain : allChildDomains) {
3215+
relatedDomainIds.add(childDomain.getId());
3216+
}
3217+
while (domainTreeNode.getParent() != null) {
3218+
domainTreeNode = _domainDao.findById(domainTreeNode.getParent());
3219+
relatedDomainIds.add(domainTreeNode.getId());
3220+
}
31973221
}
31983222

31993223
if (!isIso) {
@@ -3207,15 +3231,28 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
32073231
}
32083232
}
32093233

3234+
SearchCriteria<TemplateJoinVO> sc_public = _templateJoinDao.createSearchCriteria();
3235+
if (publicTemplates) {
3236+
sc_public.addAnd("publicTemplate", SearchCriteria.Op.EQ, true);
3237+
3238+
if (RestrictPublicTemplateAccessToDomain.value() && !relatedDomainIds.isEmpty()) {
3239+
SearchCriteria<TemplateJoinVO> sc_domains = _templateJoinDao.createSearchCriteria();
3240+
sc_domains.addOr("domainId", SearchCriteria.Op.IN, relatedDomainIds.toArray());
3241+
sc_domains.addOr("domainId", SearchCriteria.Op.NULL);
3242+
3243+
sc_public.addAnd("domainId", SearchCriteria.Op.SC, sc_domains);
3244+
}
3245+
}
3246+
32103247
// control different template filters
32113248
if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) {
3212-
sc.addAnd("publicTemplate", SearchCriteria.Op.EQ, true);
3249+
sc.addAnd("publicTemplate", SearchCriteria.Op.SC, sc_public);
32133250
if (templateFilter == TemplateFilter.featured) {
32143251
sc.addAnd("featured", SearchCriteria.Op.EQ, true);
32153252
} else {
32163253
sc.addAnd("featured", SearchCriteria.Op.EQ, false);
32173254
}
3218-
if (!permittedAccounts.isEmpty()) {
3255+
if (!permittedAccounts.isEmpty() && !RestrictPublicTemplateAccessToDomain.value()) {
32193256
SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
32203257
scc.addOr("domainId", SearchCriteria.Op.IN, relatedDomainIds.toArray());
32213258
scc.addOr("domainId", SearchCriteria.Op.NULL);
@@ -3230,14 +3267,14 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP
32303267
sc.addAnd("sharedAccountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
32313268
} else if (templateFilter == TemplateFilter.executable) {
32323269
SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
3233-
scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
3270+
scc.addAnd("publicTemplate", SearchCriteria.Op.SC, sc_public);
32343271
if (!permittedAccounts.isEmpty()) {
32353272
scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
32363273
}
32373274
sc.addAnd("publicTemplate", SearchCriteria.Op.SC, scc);
32383275
} else if (templateFilter == TemplateFilter.all && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
32393276
SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
3240-
scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
3277+
scc.addOr("publicTemplate", SearchCriteria.Op.SC, sc_public);
32413278

32423279
if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
32433280
scc.addOr("domainPath", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
@@ -3708,6 +3745,6 @@ public String getConfigComponentName() {
37083745

37093746
@Override
37103747
public ConfigKey<?>[] getConfigKeys() {
3711-
return new ConfigKey<?>[] {AllowUserViewDestroyedVM};
3748+
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, RestrictPublicTemplateAccessToDomain};
37123749
}
37133750
}

‎server/src/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@
529529
import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
530530
import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
531531
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
532+
import org.apache.cloudstack.query.QueryService;
532533
import org.apache.cloudstack.resourcedetail.dao.GuestOsDetailsDao;
533534
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
534535
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
@@ -3429,15 +3430,16 @@ public Map<String, Object> listCapabilities(final ListCapabilitiesCmd cmd) {
34293430
final long diskOffMaxSize = VolumeOrchestrationService.CustomDiskOfferingMaxSize.value();
34303431
KVMSnapshotEnabled = Boolean.parseBoolean(_configDao.getValue("KVM.snapshot.enabled"));
34313432

3432-
final boolean userPublicTemplateEnabled = TemplateManager.AllowPublicUserTemplates.valueIn(caller.getId());
3433+
boolean isAdmin = _accountService.isAdmin(caller.getId());
3434+
final boolean userPublicTemplateEnabled = (TemplateManager.AllowPublicUserTemplates.valueIn(caller.getId()) | (isAdmin && QueryService.RestrictPublicTemplateAccessToDomain.value()));
34333435

34343436
// add some parameters UI needs to handle API throttling
34353437
final boolean apiLimitEnabled = Boolean.parseBoolean(_configDao.getValue(Config.ApiLimitEnabled.key()));
34363438
final Integer apiLimitInterval = Integer.valueOf(_configDao.getValue(Config.ApiLimitInterval.key()));
34373439
final Integer apiLimitMax = Integer.valueOf(_configDao.getValue(Config.ApiLimitMax.key()));
34383440

3439-
final boolean allowUserViewDestroyedVM = (QueryManagerImpl.AllowUserViewDestroyedVM.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId()));
3440-
final boolean allowUserExpungeRecoverVM = (UserVmManager.AllowUserExpungeRecoverVm.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId()));
3441+
final boolean allowUserViewDestroyedVM = (QueryManagerImpl.AllowUserViewDestroyedVM.valueIn(caller.getId()) | isAdmin);
3442+
final boolean allowUserExpungeRecoverVM = (UserVmManager.AllowUserExpungeRecoverVm.valueIn(caller.getId()) | isAdmin);
34413443

34423444
// check if region-wide secondary storage is used
34433445
boolean regionSecondaryEnabled = false;

‎server/src/com/cloud/template/TemplateAdapterBase.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import javax.inject.Inject;
2525

2626
import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
27+
import org.apache.cloudstack.query.QueryService;
2728
import org.apache.commons.collections.CollectionUtils;
2829
import org.apache.log4j.Logger;
2930

@@ -171,15 +172,15 @@ public TemplateProfile prepare(boolean isIso, long userId, String name, String d
171172
sshkeyEnabled = Boolean.FALSE;
172173
}
173174

174-
boolean isAdmin = _accountMgr.isRootAdmin(templateOwner.getId());
175+
boolean isRootAdmin = _accountMgr.isRootAdmin(templateOwner.getId());
175176
boolean isRegionStore = false;
176177
List<ImageStoreVO> stores = _imgStoreDao.findRegionImageStores();
177178
if (stores != null && stores.size() > 0) {
178179
isRegionStore = true;
179180
}
180181

181-
if (!isAdmin && zoneIdList == null && !isRegionStore ) {
182-
// domain admin and user should also be able to register template on a region store
182+
if (!isRootAdmin && zoneIdList == null && !isRegionStore ) {
183+
// Root admin and user should also be able to register template on a region store
183184
throw new InvalidParameterValueException("Please specify a valid zone Id. Only admins can create templates in all zones.");
184185
}
185186

@@ -190,11 +191,14 @@ public TemplateProfile prepare(boolean isIso, long userId, String name, String d
190191

191192
// check whether owner can create public templates
192193
boolean allowPublicUserTemplates = TemplateManager.AllowPublicUserTemplates.valueIn(templateOwner.getId());
193-
if (!isAdmin && !allowPublicUserTemplates && isPublic) {
194+
boolean isAdmin = _accountMgr.isAdmin(templateOwner.getId());
195+
boolean isAdminWhoCanUploadPublicTemplates = isRootAdmin || (isAdmin && QueryService.RestrictPublicTemplateAccessToDomain.value());
196+
197+
if (!isAdminWhoCanUploadPublicTemplates && !allowPublicUserTemplates && isPublic) {
194198
throw new InvalidParameterValueException("Only private templates/ISO can be created.");
195199
}
196200

197-
if (!isAdmin || featured == null) {
201+
if (!isAdminWhoCanUploadPublicTemplates || featured == null) {
198202
featured = Boolean.FALSE;
199203
}
200204

‎ui/scripts/sharedFunctions.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,9 @@ cloudStack.preFilter = {
12621262
args.$form.find('.form-item[rel=isPublic]').css('display', 'inline-block');
12631263
args.$form.find('.form-item[rel=isFeatured]').css('display', 'inline-block');
12641264
args.$form.find('.form-item[rel=isrouting]').css('display', 'inline-block');
1265+
} else if (isDomainAdmin() && g_userPublicTemplateEnabled === "true") {
1266+
args.$form.find('.form-item[rel=isPublic]').css('display', 'inline-block');
1267+
args.$form.find('.form-item[rel=isFeatured]').css('display', 'inline-block');
12651268
} else {
12661269
if (g_userPublicTemplateEnabled == "true") {
12671270
args.$form.find('.form-item[rel=isPublic]').css('display', 'inline-block');

0 commit comments

Comments
 (0)