diff --git a/api/src/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java b/api/src/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java index 3de0e4f72d7e..f23e03a14373 100644 --- a/api/src/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java @@ -40,7 +40,7 @@ import com.cloud.uservm.UserVm; import com.cloud.vm.VirtualMachine; -@APICommand(name = "destroyVirtualMachine", description = "Destroys a virtual machine. Once destroyed, only the administrator can recover it.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, +@APICommand(name = "destroyVirtualMachine", description = "Destroys a virtual machine.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) public class DestroyVMCmd extends BaseAsyncCmd { @@ -59,7 +59,7 @@ public class DestroyVMCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.EXPUNGE, type = CommandType.BOOLEAN, - description = "If true is passed, the vm is expunged immediately. False by default. Parameter can be passed to the call by ROOT/Domain admin only", + description = "If true is passed, the vm is expunged immediately. False by default.", since = "4.2.1") private Boolean expunge; diff --git a/api/src/org/apache/cloudstack/query/QueryService.java b/api/src/org/apache/cloudstack/query/QueryService.java index 0013be8da3c5..95acaea44144 100644 --- a/api/src/org/apache/cloudstack/query/QueryService.java +++ b/api/src/org/apache/cloudstack/query/QueryService.java @@ -69,6 +69,7 @@ import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.api.response.VolumeResponse; import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.cloudstack.framework.config.ConfigKey; import com.cloud.exception.PermissionDeniedException; @@ -77,6 +78,9 @@ * */ public interface QueryService { + // Config keys + static final ConfigKey AllowUserViewDestroyedVM = new ConfigKey("Advanced", Boolean.class, "allow.user.view.destroyed.vm", "false", + "Determines whether users can view their destroyed or expunging vm ", true, ConfigKey.Scope.Account); public ListResponse searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException; @@ -131,5 +135,4 @@ public ListResponse listAffinityGroups(Long affinityGroup ListResponse searchForInternalLbVms(ListInternalLBVMsCmd cmd); public ListResponse searchForStorageTags(ListStorageTagsCmd cmd); public ListResponse searchForHostTags(ListHostTagsCmd cmd); - } diff --git a/client/tomcatconf/commands.properties.in b/client/tomcatconf/commands.properties.in index a66a3dce4863..2bc518d08a1e 100644 --- a/client/tomcatconf/commands.properties.in +++ b/client/tomcatconf/commands.properties.in @@ -82,8 +82,8 @@ scaleVirtualMachine=15 assignVirtualMachine=7 migrateVirtualMachine=1 migrateVirtualMachineWithVolume=1 -recoverVirtualMachine=7 -expungeVirtualMachine=7 +recoverVirtualMachine=15 +expungeVirtualMachine=15 getVirtualMachineUserData=15 #### snapshot commands diff --git a/framework/config/src/org/apache/cloudstack/framework/config/ConfigKey.java b/framework/config/src/org/apache/cloudstack/framework/config/ConfigKey.java index d39bb5d3562c..3d1b0e39c0d7 100644 --- a/framework/config/src/org/apache/cloudstack/framework/config/ConfigKey.java +++ b/framework/config/src/org/apache/cloudstack/framework/config/ConfigKey.java @@ -26,7 +26,6 @@ /** * ConfigKey supplants the original Config.java. It is just a class * declaration where others can declare their config variables. - * */ public class ConfigKey { diff --git a/framework/db/src/com/cloud/utils/db/SearchCriteria.java b/framework/db/src/com/cloud/utils/db/SearchCriteria.java index b7f8a93b2c84..1a8f7a3ee87d 100755 --- a/framework/db/src/com/cloud/utils/db/SearchCriteria.java +++ b/framework/db/src/com/cloud/utils/db/SearchCriteria.java @@ -32,11 +32,27 @@ */ public class SearchCriteria { public enum Op { - GT(" > ? ", 1), GTEQ(" >= ? ", 1), LT(" < ? ", 1), LTEQ(" <= ? ", 1), EQ(" = ? ", 1), NEQ(" != ? ", 1), BETWEEN(" BETWEEN ? AND ? ", 2), NBETWEEN( - " NOT BETWEEN ? AND ? ", - 2), IN(" IN () ", -1), NOTIN(" NOT IN () ", -1), LIKE(" LIKE ? ", 1), NLIKE(" NOT LIKE ? ", 1), NIN(" NOT IN () ", -1), NULL(" IS NULL ", 0), NNULL( - " IS NOT NULL ", - 0), SC(" () ", 1), TEXT(" () ", 1), RP("", 0), AND(" AND ", 0), OR(" OR ", 0), NOT(" NOT ", 0); + GT(" > ? ", 1), + GTEQ(" >= ? ", 1), + LT(" < ? ", 1), + LTEQ(" <= ? ", 1), + EQ(" = ? ", 1), + NEQ(" != ? ", 1), + BETWEEN(" BETWEEN ? AND ? ", 2), + NBETWEEN(" NOT BETWEEN ? AND ? ", 2), + IN(" IN () ", -1), + NOTIN(" NOT IN () ", -1), + LIKE(" LIKE ? ", 1), + NLIKE(" NOT LIKE ? ", 1), + NIN(" NOT IN () ", -1), + NULL(" IS NULL ", 0), + NNULL(" IS NOT NULL ", 0), + SC(" () ", 1), + TEXT(" () ", 1), + RP("", 0), + AND(" AND ", 0), + OR(" OR ", 0), + NOT(" NOT ", 0); private final String op; int params; diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java index 62714eafab20..ae625e753c60 100644 --- a/server/src/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/com/cloud/api/query/QueryManagerImpl.java @@ -100,6 +100,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateState; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.query.QueryService; import org.apache.log4j.Logger; @@ -218,7 +220,7 @@ @Component @Local(value = {QueryService.class}) -public class QueryManagerImpl extends ManagerBase implements QueryService { +public class QueryManagerImpl extends ManagerBase implements QueryService, Configurable { public static final Logger s_logger = Logger.getLogger(QueryManagerImpl.class); @@ -953,8 +955,8 @@ private Pair, Integer> searchForUserVMsInternal(ListVMsCmd cm sc.setParameters("hypervisorType", hypervisor); } - // Don't show Destroyed and Expunging vms to the end user - if (!isAdmin) { + // Don't show Destroyed and Expunging vms to the end user if the AllowUserViewDestroyedVM flag is not set. + if (!isAdmin && !AllowUserViewDestroyedVM.valueIn(caller.getAccountId())) { sc.setParameters("stateNIN", "Destroyed", "Expunging"); } @@ -973,20 +975,20 @@ private Pair, Integer> searchForUserVMsInternal(ListVMsCmd cm if (cmd instanceof ListVMsCmdByAdmin) { ListVMsCmdByAdmin aCmd = (ListVMsCmdByAdmin)cmd; if (aCmd.getPodId() != null) { - sc.setParameters("podId", pod); + sc.setParameters("podId", pod); - if (state == null) { - sc.setParameters("stateNEQ", "Destroyed"); + if (state == null) { + sc.setParameters("stateNEQ", "Destroyed"); + } } - } - if (hostId != null) { - sc.setParameters("hostIdEQ", hostId); - } + if (hostId != null) { + sc.setParameters("hostIdEQ", hostId); + } - if (storageId != null) { - sc.setParameters("poolId", storageId); - } + if (storageId != null) { + sc.setParameters("poolId", storageId); + } } if (!isRootAdmin) { @@ -3573,4 +3575,14 @@ protected ResourceDetailResponse createResourceDetailsResponse(ResourceDetail re return resourceDetailResponse; } + @Override + public String getConfigComponentName() { + return QueryService.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {AllowUserViewDestroyedVM}; + } + } diff --git a/server/src/com/cloud/vm/UserVmManager.java b/server/src/com/cloud/vm/UserVmManager.java index 324547f47ea5..618884150053 100755 --- a/server/src/com/cloud/vm/UserVmManager.java +++ b/server/src/com/cloud/vm/UserVmManager.java @@ -42,8 +42,12 @@ */ public interface UserVmManager extends UserVmService { static final String EnableDynamicallyScaleVmCK = "enable.dynamic.scale.vm"; + static final String AllowUserExpungeRecoverVmCK ="allow.user.expunge.recover.vm"; + static final ConfigKey EnableDynamicallyScaleVm = new ConfigKey("Advanced", Boolean.class, EnableDynamicallyScaleVmCK, "false", "Enables/Disables dynamically scaling a vm", true, ConfigKey.Scope.Zone); + static final ConfigKey AllowUserExpungeRecoverVm = new ConfigKey("Advanced", Boolean.class, AllowUserExpungeRecoverVmCK, "false", + "Determines whether users can expunge or recover their vm", true, ConfigKey.Scope.Account); static final int MAX_USER_DATA_LENGTH_BYTES = 2048; diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 66f7fc1d8d6d..9be10ef573ad 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -1565,6 +1565,7 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE final Long vmId = cmd.getId(); Account caller = CallContext.current().getCallingAccount(); + Long userId = caller.getAccountId(); // Verify input parameters final UserVmVO vm = _vmDao.findById(vmId); @@ -1573,8 +1574,10 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); } - // check permissions - _accountMgr.checkAccess(caller, null, true, vm); + // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVm is false for the caller. + if (!_accountMgr.isAdmin(userId) && !AllowUserExpungeRecoverVm.valueIn(userId)) { + throw new PermissionDeniedException("Recovering a vm can only be done by an Admin."); + } if (vm.getRemoved() != null) { if (s_logger.isDebugEnabled()) { @@ -2176,7 +2179,8 @@ public UserVm destroyVm(DestroyVMCmd cmd) throws ResourceUnavailableException, C long vmId = cmd.getId(); boolean expunge = cmd.getExpunge(); - if (!_accountMgr.isAdmin(ctx.getCallingAccount().getId()) && expunge) { + // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVm is false for the caller. + if (expunge && !_accountMgr.isAdmin(ctx.getCallingAccount().getId()) && !AllowUserExpungeRecoverVm.valueIn(cmd.getEntityOwnerId())) { throw new PermissionDeniedException("Parameter " + ApiConstants.EXPUNGE + " can be passed by Admin only"); } @@ -3802,7 +3806,10 @@ public UserVm expungeVm(long vmId) throws ResourceUnavailableException, Concurre throw ex; } - _accountMgr.checkAccess(caller, null, true, vm); + // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVm is false for the caller. + if (!_accountMgr.isAdmin(userId) && !AllowUserExpungeRecoverVm.valueIn(userId)) { + throw new PermissionDeniedException("Expunging a vm can only be done by an Admin."); + } boolean status; @@ -5021,7 +5028,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {EnableDynamicallyScaleVm}; + return new ConfigKey[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm}; } @Override