Skip to content

Allow users to share templates with Accounts or Projects through the #3241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PaulAngus merged 9 commits into from
Jul 18, 2019
Merged

Allow users to share templates with Accounts or Projects through the #3241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b36eb37
Allow users to share templates with Accounts or Projects through the
Jan 16, 2019
0950b7f
Allow project id to be passed in even if no account name is specified
Apr 15, 2019
0fadb1d
Remove unused import
Jun 4, 2019
074e2ee
Fixed typo in capabilities key
Jun 10, 2019
4c219a5
Update dialogue layout and population logic for permissions of template
Jun 13, 2019
4facf03
Change behaviour to show only supported projects and accounts with up…
Jun 21, 2019
1555d31
Allow admins to see accounts dropdown and only hide lists for users
Jun 24, 2019
d383aa3
Fix bug for hover animation
Jun 24, 2019
e90bad4
Don't allow sharing project owned templates as you cannot retrieve th…
Jul 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoPermissionsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected String getResponseName() {
@Parameter(name = ApiConstants.ACCOUNTS,
type = CommandType.LIST,
collectionType = CommandType.STRING,
description = "a comma delimited list of accounts. If specified, \"op\" parameter has to be passed in.")
description = "a comma delimited list of accounts within caller's domain. If specified, \"op\" parameter has to be passed in.")
private List<String> accountNames;

@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = TemplateResponse.class, required = true, description = "the template ID")
Expand Down Expand Up @@ -80,7 +80,6 @@ public List<String> getAccountNames() {
if (accountNames != null && projectIds != null) {
throw new InvalidParameterValueException("Accounts and projectIds can't be specified together");
}

return accountNames;
}

Expand Down
1 change: 1 addition & 0 deletions api/src/main/java/org/apache/cloudstack/api/command/user/config/ListCapabilitiesCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public void execute() {
response.setKVMSnapshotEnabled((Boolean)capabilities.get("KVMSnapshotEnabled"));
response.setAllowUserViewDestroyedVM((Boolean)capabilities.get("allowUserViewDestroyedVM"));
response.setAllowUserExpungeRecoverVM((Boolean)capabilities.get("allowUserExpungeRecoverVM"));
response.setAllowUserViewAllDomainAccounts((Boolean)capabilities.get("allowUserViewAllDomainAccounts"));
if (capabilities.containsKey("apiLimitInterval")) {
response.setApiLimitInterval((Integer)capabilities.get("apiLimitInterval"));
}
Expand Down
8 changes: 8 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ public class CapabilitiesResponse extends BaseResponse {
@Param(description = "true if the user can recover and expunge virtualmachines, false otherwise", since = "4.6.0")
private boolean allowUserExpungeRecoverVM;

@SerializedName("allowuserviewalldomainaccounts")
@Param(description = "true if users can see all accounts within the same domain, false otherwise")
private boolean allowUserViewAllDomainAccounts;

public void setSecurityGroupsEnabled(boolean securityGroupsEnabled) {
this.securityGroupsEnabled = securityGroupsEnabled;
}
Expand Down Expand Up @@ -143,4 +147,8 @@ public void setAllowUserViewDestroyedVM(boolean allowUserViewDestroyedVM) {
public void setAllowUserExpungeRecoverVM(boolean allowUserExpungeRecoverVM) {
this.allowUserExpungeRecoverVM = allowUserExpungeRecoverVM;
}

public void setAllowUserViewAllDomainAccounts(boolean allowUserViewAllDomainAccounts) {
this.allowUserViewAllDomainAccounts = allowUserViewAllDomainAccounts;
}
}
4 changes: 4 additions & 0 deletions api/src/main/java/org/apache/cloudstack/query/QueryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ public interface QueryService {
"network offering, zones), we use the flag to determine if the entities should be sorted ascending (when flag is true) " +
"or descending (when flag is false). Within the scope of the config all users see the same result.", true, ConfigKey.Scope.Global);

public static final ConfigKey<Boolean> AllowUserViewAllDomainAccounts = new ConfigKey<>("Advanced", Boolean.class,
"allow.user.view.all.domain.accounts", "false",
"Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);

ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;

ListResponse<EventResponse> searchForEvents(ListEventsCmd cmd);
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/com/cloud/api/ApiResponseHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,11 @@ public TemplatePermissionsResponse createTemplatePermissionsResponse(ResponseVie
List<String> regularAccounts = new ArrayList<String>();
for (String accountName : accountNames) {
Account account = ApiDBUtils.findAccountByNameDomain(accountName, templateOwner.getDomainId());
if (account == null) {
s_logger.error("Missing Account " + accountName + " in domain " + templateOwner.getDomainId());
continue;
}

if (account.getType() != Account.ACCOUNT_TYPE_PROJECT) {
regularAccounts.add(accountName);
} else {
Expand Down
13 changes: 11 additions & 2 deletions server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
* com.cloud.api.query.QueryService#searchForUsers(org.apache.cloudstack
* .api.command.admin.user.ListUsersCmd)
*/

@Override
public ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException {
Pair<List<UserAccountJoinVO>, Integer> result = searchForUsersInternal(cmd);
Expand Down Expand Up @@ -1961,7 +1962,8 @@ private Pair<List<AccountJoinVO>, Integer> searchForAccountsInternal(ListAccount
// if no "id" specified...
if (accountId == null) {
// listall only has significance if they are an admin
if (listAll && callerIsAdmin) {
boolean isDomainListAllAllowed = AllowUserViewAllDomainAccounts.valueIn(caller.getDomainId());
if ((listAll && callerIsAdmin) || isDomainListAllAllowed) {
// if no domain id specified, use caller's domain
if (domainId == null) {
domainId = caller.getDomainId();
Expand Down Expand Up @@ -2007,6 +2009,7 @@ private Pair<List<AccountJoinVO>, Integer> searchForAccountsInternal(ListAccount
sb.and("needsCleanup", sb.entity().isNeedsCleanup(), SearchCriteria.Op.EQ);
sb.and("typeNEQ", sb.entity().getType(), SearchCriteria.Op.NEQ);
sb.and("idNEQ", sb.entity().getId(), SearchCriteria.Op.NEQ);
sb.and("type2NEQ", sb.entity().getType(), SearchCriteria.Op.NEQ);

if (domainId != null && isRecursive) {
sb.and("path", sb.entity().getDomainPath(), SearchCriteria.Op.LIKE);
Expand All @@ -2016,9 +2019,15 @@ private Pair<List<AccountJoinVO>, Integer> searchForAccountsInternal(ListAccount

// don't return account of type project to the end user
sc.setParameters("typeNEQ", Account.ACCOUNT_TYPE_PROJECT);

// don't return system account...
sc.setParameters("idNEQ", Account.ACCOUNT_ID_SYSTEM);

// do not return account of type domain admin to the end user
if (!callerIsAdmin) {
sc.setParameters("type2NEQ", Account.ACCOUNT_TYPE_DOMAIN_ADMIN);
}

if (keyword != null) {
SearchCriteria<AccountJoinVO> ssc = _accountJoinDao.createSearchCriteria();
ssc.addOr("accountName", SearchCriteria.Op.LIKE, "%" + keyword + "%");
Expand Down Expand Up @@ -3761,6 +3770,6 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMBlacklistedDetails, UserVMReadOnlyUIDetails, SortKeyAscending};
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMBlacklistedDetails, UserVMReadOnlyUIDetails, SortKeyAscending, AllowUserViewAllDomainAccounts};
}
}
6 changes: 3 additions & 3 deletions server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@
import org.apache.cloudstack.api.response.SecurityGroupResponse;
import org.apache.cloudstack.api.response.UserVmResponse;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.query.QueryService;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;

import com.cloud.api.ApiDBUtils;
import com.cloud.api.ApiResponseHelper;
import com.cloud.api.query.QueryManagerImpl;
import com.cloud.api.query.vo.UserVmJoinVO;
import com.cloud.gpu.GPU;
import com.cloud.service.ServiceOfferingDetailsVO;
Expand Down Expand Up @@ -315,14 +315,14 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us
}
// Remove blacklisted settings if user is not admin
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
String[] userVmSettingsToHide = QueryManagerImpl.UserVMBlacklistedDetails.value().split(",");
String[] userVmSettingsToHide = QueryService.UserVMBlacklistedDetails.value().split(",");
for (String key : userVmSettingsToHide) {
resourceDetails.remove(key.trim());
}
}
userVmResponse.setDetails(resourceDetails);
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
userVmResponse.setReadOnlyUIDetails(QueryManagerImpl.UserVMReadOnlyUIDetails.value());
userVmResponse.setReadOnlyUIDetails(QueryService.UserVMReadOnlyUIDetails.value());
}
}

Expand Down
7 changes: 5 additions & 2 deletions server/src/main/java/com/cloud/server/ManagementServerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@
import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.cloudstack.query.QueryService;
import org.apache.cloudstack.resourcedetail.dao.GuestOsDetailsDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
Expand All @@ -554,7 +555,6 @@
import com.cloud.alert.AlertVO;
import com.cloud.alert.dao.AlertDao;
import com.cloud.api.ApiDBUtils;
import com.cloud.api.query.QueryManagerImpl;
import com.cloud.capacity.Capacity;
import com.cloud.capacity.CapacityVO;
import com.cloud.capacity.dao.CapacityDao;
Expand Down Expand Up @@ -3484,9 +3484,11 @@ public Map<String, Object> listCapabilities(final ListCapabilitiesCmd cmd) {
final Integer apiLimitInterval = Integer.valueOf(_configDao.getValue(Config.ApiLimitInterval.key()));
final Integer apiLimitMax = Integer.valueOf(_configDao.getValue(Config.ApiLimitMax.key()));

final boolean allowUserViewDestroyedVM = (QueryManagerImpl.AllowUserViewDestroyedVM.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId()));
final boolean allowUserViewDestroyedVM = (QueryService.AllowUserViewDestroyedVM.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId()));
final boolean allowUserExpungeRecoverVM = (UserVmManager.AllowUserExpungeRecoverVm.valueIn(caller.getId()) | _accountService.isAdmin(caller.getId()));

final boolean allowUserViewAllDomainAccounts = (QueryService.AllowUserViewAllDomainAccounts.valueIn(caller.getDomainId()));

// check if region-wide secondary storage is used
boolean regionSecondaryEnabled = false;
final List<ImageStoreVO> imgStores = _imgStoreDao.findRegionImageStores();
Expand All @@ -3506,6 +3508,7 @@ public Map<String, Object> listCapabilities(final ListCapabilitiesCmd cmd) {
capabilities.put("KVMSnapshotEnabled", KVMSnapshotEnabled);
capabilities.put("allowUserViewDestroyedVM", allowUserViewDestroyedVM);
capabilities.put("allowUserExpungeRecoverVM", allowUserExpungeRecoverVM);
capabilities.put("allowUserViewAllDomainAccounts", allowUserViewAllDomainAccounts);
if (apiLimitEnabled) {
capabilities.put("apiLimitInterval", apiLimitInterval);
capabilities.put("apiLimitMax", apiLimitMax);
Expand Down
74 changes: 39 additions & 35 deletions server/src/main/java/com/cloud/template/TemplateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,14 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.deploy.DeployDestination;
import com.cloud.storage.ImageStoreUploadMonitorImpl;
import com.cloud.utils.StringUtils;
import com.cloud.utils.EncryptionUtil;
import com.cloud.utils.DateUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.EnumUtils;
import com.cloud.vm.VmDetailConstants;
import com.google.common.base.Joiner;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;

import org.apache.cloudstack.api.command.user.iso.GetUploadParamsForIsoCmd;
import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
import org.apache.cloudstack.framework.async.AsyncCallFuture;
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseListTemplateOrIsoPermissionsCmd;
import org.apache.cloudstack.api.BaseUpdateTemplateOrIsoCmd;
import org.apache.cloudstack.api.BaseUpdateTemplateOrIsoPermissionsCmd;
import org.apache.cloudstack.api.command.user.iso.DeleteIsoCmd;
import org.apache.cloudstack.api.command.user.iso.ExtractIsoCmd;
import org.apache.cloudstack.api.command.user.iso.GetUploadParamsForIsoCmd;
import org.apache.cloudstack.api.command.user.iso.ListIsoPermissionsCmd;
import org.apache.cloudstack.api.command.user.iso.RegisterIsoCmd;
import org.apache.cloudstack.api.command.user.iso.UpdateIsoCmd;
Expand All @@ -69,6 +48,7 @@
import org.apache.cloudstack.api.command.user.template.CreateTemplateCmd;
import org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd;
import org.apache.cloudstack.api.command.user.template.ExtractTemplateCmd;
import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
import org.apache.cloudstack.api.command.user.template.ListTemplatePermissionsCmd;
import org.apache.cloudstack.api.command.user.template.RegisterTemplateCmd;
import org.apache.cloudstack.api.command.user.template.UpdateTemplateCmd;
Expand All @@ -95,6 +75,7 @@
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
import org.apache.cloudstack.framework.async.AsyncCallFuture;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
Expand All @@ -104,13 +85,22 @@
import org.apache.cloudstack.storage.command.AttachCommand;
import org.apache.cloudstack.storage.command.CommandResult;
import org.apache.cloudstack.storage.command.DettachCommand;
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
import org.apache.cloudstack.storage.to.TemplateObjectTO;
import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.log4j.Logger;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

import com.cloud.agent.AgentManager;
import com.cloud.agent.api.Answer;
Expand All @@ -129,6 +119,7 @@
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.deploy.DeployDestination;
import com.cloud.domain.Domain;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.ActionEvent;
Expand All @@ -148,6 +139,7 @@
import com.cloud.projects.ProjectManager;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.GuestOSVO;
import com.cloud.storage.ImageStoreUploadMonitorImpl;
import com.cloud.storage.LaunchPermissionVO;
import com.cloud.storage.Snapshot;
import com.cloud.storage.SnapshotVO;
Expand Down Expand Up @@ -186,6 +178,11 @@
import com.cloud.user.ResourceLimitService;
import com.cloud.user.dao.AccountDao;
import com.cloud.uservm.UserVm;
import com.cloud.utils.DateUtil;
import com.cloud.utils.EncryptionUtil;
import com.cloud.utils.EnumUtils;
import com.cloud.utils.Pair;
import com.cloud.utils.StringUtils;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.concurrency.NamedThreadFactory;
Expand All @@ -200,11 +197,12 @@
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import com.google.common.base.Joiner;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;

public class TemplateManagerImpl extends ManagerBase implements TemplateManager, TemplateApiService, Configurable {
private final static Logger s_logger = Logger.getLogger(TemplateManagerImpl.class);
Expand Down Expand Up @@ -1483,9 +1481,24 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions
throw new InvalidParameterValueException("unable to update permissions for " + mediaType + " with id " + id);
}

boolean isAdmin = _accountMgr.isAdmin(caller.getId());
Long ownerId = template.getAccountId();
Account owner = _accountMgr.getAccount(ownerId);
if (ownerId == null) {
// if there is no owner of the template then it's probably already a
// public template (or domain private template) so
// publishing to individual users is irrelevant
throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName());
}

if (owner.getType() == Account.ACCOUNT_TYPE_PROJECT) {
// Currently project owned templates cannot be shared outside project but is available to all users within project by default.
throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName() +
". Project owned templates cannot be shared outside template.");
}

// check configuration parameter(allow.public.user.templates) value for
// the template owner
boolean isAdmin = _accountMgr.isAdmin(caller.getId());
boolean allowPublicUserTemplates = AllowPublicUserTemplates.valueIn(template.getAccountId());
if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) {
throw new InvalidParameterValueException("Only private " + mediaType + "s can be created.");
Expand All @@ -1499,14 +1512,6 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions
}
}

Long ownerId = template.getAccountId();
if (ownerId == null) {
// if there is no owner of the template then it's probably already a
// public template (or domain private template) so
// publishing to individual users is irrelevant
throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName());
}

//Only admin or owner of the template should be able to change its permissions
if (caller.getId() != ownerId && !isAdmin) {
throw new InvalidParameterValueException("Unable to grant permission to account " + caller.getAccountName() + " as it is neither admin nor owner or the template");
Expand Down Expand Up @@ -1540,7 +1545,6 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions
}

//Derive the domain id from the template owner as updateTemplatePermissions is not cross domain operation
Account owner = _accountMgr.getAccount(ownerId);
final Domain domain = _domainDao.findById(owner.getDomainId());
if ("add".equalsIgnoreCase(operation)) {
final List<String> accountNamesFinal = accountNames;
Expand Down
8 changes: 8 additions & 0 deletions ui/css/cloudstack3.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading