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 3de0e4f72d7..f23e03a1437 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.user.Account; 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 0cf05a643f1..1a5ac11b299 100644 --- a/api/src/org/apache/cloudstack/query/QueryService.java +++ b/api/src/org/apache/cloudstack/query/QueryService.java @@ -71,6 +71,7 @@ import org.apache.cloudstack.api.response.UserResponse; 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; @@ -80,6 +81,10 @@ import com.cloud.exception.PermissionDeniedException; */ 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); + ListResponse searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException; ListResponse searchForEvents(ListEventsCmd cmd); diff --git a/client/tomcatconf/commands.properties.in b/client/tomcatconf/commands.properties.in index 29752c62995..c32ecc4bdfb 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/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java index 8c304b99347..6994b27327b 100644 --- a/server/src/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/com/cloud/api/query/QueryManagerImpl.java @@ -103,6 +103,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; 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; @@ -223,7 +225,7 @@ import com.cloud.vm.dao.UserVmDetailsDao; @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); @@ -979,8 +981,8 @@ public class QueryManagerImpl extends ManagerBase implements QueryService { 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"); } @@ -3685,4 +3687,13 @@ public class QueryManagerImpl extends ManagerBase implements QueryService { 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 324547f47ea..5d9a6614f2c 100644 --- a/server/src/com/cloud/vm/UserVmManager.java +++ b/server/src/com/cloud/vm/UserVmManager.java @@ -42,8 +42,11 @@ import com.cloud.utils.Pair; */ 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 b9c5350f236..a3f4b82c77d 100644 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -1705,6 +1705,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir final Long vmId = cmd.getId(); Account caller = CallContext.current().getCallingAccount(); + final Long userId = caller.getAccountId(); // Verify input parameters final UserVmVO vm = _vmDao.findById(vmId); @@ -1713,8 +1714,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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. Or when the allow.user.expunge.recover.vm key is set."); + } if (vm.getRemoved() != null) { if (s_logger.isDebugEnabled()) { @@ -2404,8 +2407,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir long vmId = cmd.getId(); boolean expunge = cmd.getExpunge(); - if (!_accountMgr.isAdmin(ctx.getCallingAccount().getId()) && expunge) { - throw new PermissionDeniedException("Parameter " + ApiConstants.EXPUNGE + " can be passed by Admin only"); + // 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. Or when the allow.user.expunge.recover.vm key is set."); } UserVm destroyedVm = destroyVm(vmId); @@ -4072,7 +4076,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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. Or when the allow.user.expunge.recover.vm key is set."); + } boolean status; @@ -5292,7 +5299,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {EnableDynamicallyScaleVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax}; + return new ConfigKey[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax}; } @Override