From ebaf064d92312e3710f67ef96c6fd540328fa78c Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 6 Sep 2024 10:45:28 +0530 Subject: [PATCH 01/10] Fix root disk resize (don't allow) when service offering has root disk size, only allow through service offering change (#9428) --- .../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 7 +------ .../java/com/cloud/storage/VolumeApiServiceImplTest.java | 7 ++----- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index f52cd155142..b506858b237 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -126,8 +126,6 @@ import com.cloud.agent.api.ModifyTargetsCommand; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; import com.cloud.api.ApiDBUtils; -import com.cloud.api.query.dao.ServiceOfferingJoinDao; -import com.cloud.api.query.vo.ServiceOfferingJoinVO; import com.cloud.configuration.Config; import com.cloud.configuration.ConfigurationManager; import com.cloud.configuration.Resource.ResourceType; @@ -275,8 +273,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private ServiceOfferingDetailsDao _serviceOfferingDetailsDao; @Inject - private ServiceOfferingJoinDao serviceOfferingJoinDao; - @Inject private UserVmDao _userVmDao; @Inject private UserVmDetailsDao userVmDetailsDao; @@ -1399,8 +1395,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic boolean isNotIso = format != null && format != ImageFormat.ISO; boolean isRoot = Volume.Type.ROOT.equals(volume.getVolumeType()); - ServiceOfferingJoinVO serviceOfferingView = serviceOfferingJoinDao.findById(diskOffering.getId()); - boolean isOfferingEnforcingRootDiskSize = serviceOfferingView != null && serviceOfferingView.getRootDiskSize() > 0; + boolean isOfferingEnforcingRootDiskSize = diskOffering.isComputeOnly() && diskOffering.getDiskSize() > 0; return isOfferingEnforcingRootDiskSize && isRoot && isNotIso; } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index b017a2d3371..a0f89956df5 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -84,7 +84,6 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; import com.cloud.api.query.dao.ServiceOfferingJoinDao; -import com.cloud.api.query.vo.ServiceOfferingJoinVO; import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenterVO; @@ -1365,10 +1364,8 @@ public class VolumeApiServiceImplTest { when(volume.getTemplateId()).thenReturn(1l); DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); - - ServiceOfferingJoinVO serviceOfferingJoinVO = Mockito.mock(ServiceOfferingJoinVO.class); - when(serviceOfferingJoinVO.getRootDiskSize()).thenReturn(rootDisk); - when(serviceOfferingJoinDao.findById(anyLong())).thenReturn(serviceOfferingJoinVO); + when(diskOffering.isComputeOnly()).thenReturn(true); + when(diskOffering.getDiskSize()).thenReturn(rootDisk); VMTemplateVO template = Mockito.mock(VMTemplateVO.class); when(template.getFormat()).thenReturn(imageFormat); From 3f5a77ef5802871d24777ef8a50f46b952e200a6 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Mon, 9 Sep 2024 10:01:41 +0200 Subject: [PATCH 02/10] Linstor: Fix migrate primary storage (#9528) --- .../kvm/storage/KVMPhysicalDisk.java | 27 ++++++++++++-- .../kvm/storage/KVMStorageProcessor.java | 5 +++ .../kvm/storage/LibvirtStorageAdaptor.java | 5 ++- .../kvm/storage/LinstorStorageAdaptor.java | 9 +++++ .../LinstorPrimaryDataStoreDriverImpl.java | 37 ++++--------------- .../storage/datastore/util/LinstorUtil.java | 24 ++++++++++++ .../snapshot/LinstorVMSnapshotStrategy.java | 4 +- 7 files changed, 76 insertions(+), 35 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java index c9abf399530..9d9a6415e27 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java @@ -18,6 +18,7 @@ package com.cloud.hypervisor.kvm.storage; import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.apache.cloudstack.utils.qemu.QemuObject; +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.lang3.StringUtils; import java.util.ArrayList; @@ -25,8 +26,10 @@ import java.util.List; public class KVMPhysicalDisk { private String path; - private String name; - private KVMStoragePool pool; + private final String name; + private final KVMStoragePool pool; + private String dispName; + private String vmName; private boolean useAsTemplate; public static String RBDStringBuilder(String monHost, int monPort, String authUserName, String authSecret, String image) { @@ -81,7 +84,9 @@ public class KVMPhysicalDisk { @Override public String toString() { - return "KVMPhysicalDisk [path=" + path + ", name=" + name + ", pool=" + pool + ", format=" + format + ", size=" + size + ", virtualSize=" + virtualSize + "]"; + return String.format("KVMPhysicalDisk %s", + ReflectionToStringBuilderUtils.reflectOnlySelectedFields( + this, "path", "name", "pool", "format", "size", "virtualSize", "dispName", "vmName")); } public void setFormat(PhysicalDiskFormat format) { @@ -135,4 +140,20 @@ public class KVMPhysicalDisk { public void setUseAsTemplate() { this.useAsTemplate = true; } public boolean useAsTemplate() { return this.useAsTemplate; } + + public String getDispName() { + return dispName; + } + + public void setDispName(String dispName) { + this.dispName = dispName; + } + + public String getVmName() { + return vmName; + } + + public void setVmName(String vmName) { + this.vmName = vmName; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index a3a79de6bf5..3b0e2e5b371 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -432,6 +432,7 @@ public class KVMStorageProcessor implements StorageProcessor { if (!storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path, details)) { s_logger.warn("Failed to connect new volume at path: " + path + ", in storage pool id: " + primaryStore.getUuid()); } + BaseVol.setDispName(template.getName()); vol = storagePoolMgr.copyPhysicalDisk(BaseVol, path != null ? path : volume.getUuid(), primaryPool, cmd.getWaitInMillSeconds(), null, volume.getPassphrase(), volume.getProvisioningType()); @@ -524,6 +525,8 @@ public class KVMStorageProcessor implements StorageProcessor { final KVMPhysicalDisk volume = secondaryStoragePool.getPhysicalDisk(srcVolumeName); volume.setFormat(PhysicalDiskFormat.valueOf(srcFormat.toString())); + volume.setDispName(srcVol.getName()); + volume.setVmName(srcVol.getVmName()); final KVMPhysicalDisk newDisk = storagePoolMgr.copyPhysicalDisk(volume, path != null ? path : volumeName, primaryPool, cmd.getWaitInMillSeconds()); @@ -2486,6 +2489,8 @@ public class KVMStorageProcessor implements StorageProcessor { } volume.setFormat(PhysicalDiskFormat.valueOf(srcFormat.toString())); + volume.setDispName(srcVol.getName()); + volume.setVmName(srcVol.getVmName()); String destVolumeName = null; if (destPrimaryStore.isManaged()) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index df6c047e7e2..cad9d429969 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1402,7 +1402,10 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { */ KVMStoragePool srcPool = disk.getPool(); - PhysicalDiskFormat sourceFormat = disk.getFormat(); + /* Linstor images are always stored as RAW, but Linstor uses qcow2 in DB, + to support snapshots(backuped) as qcow2 files. */ + PhysicalDiskFormat sourceFormat = srcPool.getType() != StoragePoolType.Linstor ? + disk.getFormat() : PhysicalDiskFormat.RAW; String sourcePath = disk.getPath(); KVMPhysicalDisk newDisk; diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index 83a84a91622..659ff7bfe53 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -516,6 +516,15 @@ public class LinstorStorageAdaptor implements StorageAdaptor { final KVMPhysicalDisk dstDisk = destPools.createPhysicalDisk( name, QemuImg.PhysicalDiskFormat.RAW, provisioningType, disk.getVirtualSize(), null); + final DevelopersApi api = getLinstorAPI(destPools); + final String rscName = LinstorUtil.RSC_PREFIX + name; + try { + LinstorUtil.applyAuxProps(api, rscName, disk.getDispName(), disk.getVmName()); + } catch (ApiException apiExc) { + s_logger.error(String.format("Error setting aux properties for %s", rscName)); + logLinstorAnswers(apiExc.getApiCallRcList()); + } + s_logger.debug(String.format("Linstor.copyPhysicalDisk: dstPath: %s", dstDisk.getPath())); final QemuImgFile destFile = new QemuImgFile(dstDisk.getPath()); destFile.setFormat(dstDisk.getFormat()); diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java index 3ca3332fcdd..27904ed441b 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java @@ -26,7 +26,6 @@ import com.linbit.linstor.api.model.ResourceDefinition; import com.linbit.linstor.api.model.ResourceDefinitionCloneRequest; import com.linbit.linstor.api.model.ResourceDefinitionCloneStarted; import com.linbit.linstor.api.model.ResourceDefinitionCreate; -import com.linbit.linstor.api.model.ResourceDefinitionModify; import com.linbit.linstor.api.model.ResourceGroupSpawn; import com.linbit.linstor.api.model.ResourceMakeAvailable; import com.linbit.linstor.api.model.Snapshot; @@ -62,8 +61,8 @@ import com.cloud.resource.ResourceState; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ResizeVolumePayload; import com.cloud.storage.SnapshotVO; -import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.Storage; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateStoragePoolVO; @@ -389,27 +388,6 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver } } - private void applyAuxProps(DevelopersApi api, String rscName, String dispName, String vmName) - throws ApiException - { - ResourceDefinitionModify rdm = new ResourceDefinitionModify(); - Properties props = new Properties(); - if (dispName != null) - { - props.put("Aux/cs-name", dispName); - } - if (vmName != null) - { - props.put("Aux/cs-vm-name", vmName); - } - if (!props.isEmpty()) - { - rdm.setOverrideProps(props); - ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm); - checkLinstorAnswersThrow(answers); - } - } - private String getRscGrp(StoragePoolVO storagePoolVO) { return storagePoolVO.getUserInfo() != null && !storagePoolVO.getUserInfo().isEmpty() ? storagePoolVO.getUserInfo() : "DfltRscGrp"; @@ -427,7 +405,8 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver ApiCallRcList answers = api.resourceGroupSpawn(rscGrp, rscGrpSpawn); checkLinstorAnswersThrow(answers); - applyAuxProps(api, rscName, volName, vmName); + answers = LinstorUtil.applyAuxProps(api, rscName, volName, vmName); + checkLinstorAnswersThrow(answers); return LinstorUtil.getDevicePath(api, rscName); } catch (ApiException apiEx) @@ -499,7 +478,7 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver resizeResource(linstorApi, rscName, volumeInfo.getSize()); } - applyAuxProps(linstorApi, rscName, volumeInfo.getName(), volumeInfo.getAttachedVmName()); + LinstorUtil.applyAuxProps(linstorApi, rscName, volumeInfo.getName(), volumeInfo.getAttachedVmName()); applyQoSSettings(storagePoolVO, linstorApi, rscName, volumeInfo.getMaxIops()); return LinstorUtil.getDevicePath(linstorApi, rscName); @@ -551,7 +530,7 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver answers = linstorApi.resourceSnapshotRestore(cloneRes, snapName, snapshotRestore); checkLinstorAnswersThrow(answers); - applyAuxProps(linstorApi, rscName, volumeVO.getName(), null); + LinstorUtil.applyAuxProps(linstorApi, rscName, volumeVO.getName(), null); applyQoSSettings(storagePoolVO, linstorApi, rscName, volumeVO.getMaxIops()); return LinstorUtil.getDevicePath(linstorApi, rscName); @@ -833,7 +812,7 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver VolumeInfo volume = sinfo.getBaseVolume(); deleteSnapshot( srcData.getDataStore(), - LinstorUtil.RSC_PREFIX + volume.getUuid(), + LinstorUtil.RSC_PREFIX + volume.getPath(), LinstorUtil.RSC_PREFIX + sinfo.getUuid()); } res = new CopyCommandResult(null, answer); @@ -969,7 +948,7 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver VolumeInfo srcVolInfo = (VolumeInfo) srcData; final StoragePoolVO pool = _storagePoolDao.findById(srcVolInfo.getDataStore().getId()); final DevelopersApi api = LinstorUtil.getLinstorAPI(pool.getHostAddress()); - final String rscName = LinstorUtil.RSC_PREFIX + srcVolInfo.getUuid(); + final String rscName = LinstorUtil.RSC_PREFIX + srcVolInfo.getPath(); VolumeObjectTO to = (VolumeObjectTO) srcVolInfo.getTO(); // patch source format @@ -1082,7 +1061,7 @@ public class LinstorPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver options.put("volumeSize", snapshotObject.getBaseVolume().getSize() + ""); try { - final String rscName = LinstorUtil.RSC_PREFIX + snapshotObject.getBaseVolume().getUuid(); + final String rscName = LinstorUtil.RSC_PREFIX + snapshotObject.getBaseVolume().getPath(); String snapshotName = setCorrectSnapshotPath(api, rscName, snapshotObject); CopyCommand cmd = new LinstorBackupSnapshotCommand( diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java index b857b4ebb83..9aa6f151abc 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java @@ -22,8 +22,10 @@ import com.linbit.linstor.api.DevelopersApi; import com.linbit.linstor.api.model.ApiCallRc; import com.linbit.linstor.api.model.ApiCallRcList; import com.linbit.linstor.api.model.Node; +import com.linbit.linstor.api.model.Properties; import com.linbit.linstor.api.model.ProviderKind; import com.linbit.linstor.api.model.Resource; +import com.linbit.linstor.api.model.ResourceDefinitionModify; import com.linbit.linstor.api.model.ResourceGroup; import com.linbit.linstor.api.model.ResourceWithVolumes; import com.linbit.linstor.api.model.StoragePool; @@ -239,4 +241,26 @@ public class LinstorUtil { s_logger.error(errMsg); throw new CloudRuntimeException("Linstor: " + errMsg); } + + public static ApiCallRcList applyAuxProps(DevelopersApi api, String rscName, String dispName, String vmName) + throws ApiException + { + ResourceDefinitionModify rdm = new ResourceDefinitionModify(); + Properties props = new Properties(); + if (dispName != null) + { + props.put("Aux/cs-name", dispName); + } + if (vmName != null) + { + props.put("Aux/cs-vm-name", vmName); + } + ApiCallRcList answers = new ApiCallRcList(); + if (!props.isEmpty()) + { + rdm.setOverrideProps(props); + answers = api.resourceDefinitionModify(rscName, rdm); + } + return answers; + } } diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java index af7b6978db5..daad3d5dc57 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java @@ -239,7 +239,7 @@ public class LinstorVMSnapshotStrategy extends DefaultVMSnapshotStrategy { final String snapshotName = vmSnapshotVO.getName(); final List failedToDelete = new ArrayList<>(); for (VolumeObjectTO volumeObjectTO : volumeTOs) { - final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getUuid(); + final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getPath(); String err = linstorDeleteSnapshot(api, rscName, snapshotName); if (err != null) @@ -292,7 +292,7 @@ public class LinstorVMSnapshotStrategy extends DefaultVMSnapshotStrategy { final String snapshotName = vmSnapshotVO.getName(); for (VolumeObjectTO volumeObjectTO : volumeTOs) { - final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getUuid(); + final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getPath(); String err = linstorRevertSnapshot(api, rscName, snapshotName); if (err != null) { throw new CloudRuntimeException(String.format( From a93f7154a0750ac2bb2527eb82445dd2bec9d84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Mon, 9 Sep 2024 09:38:42 -0300 Subject: [PATCH 03/10] fix start VMs through group action (#9652) --- ui/src/config/section/compute.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index 4c5a61e3bdc..2570d8a2651 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -120,7 +120,13 @@ export default { groupAction: true, popup: true, groupMap: (selection, values) => { return selection.map(x => { return { id: x, considerlasthost: values.considerlasthost } }) }, - args: ['considerlasthost'], + args: (record, store) => { + if (['Admin'].includes(store.userInfo.roletype)) { + return ['considerlasthost'] + } + + return [] + }, show: (record) => { return ['Stopped'].includes(record.state) }, component: shallowRef(defineAsyncComponent(() => import('@/views/compute/StartVirtualMachine.vue'))) }, From 1303a4f323dc21f23cff60a3ed83694f7d76d89c Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 9 Sep 2024 18:14:50 +0530 Subject: [PATCH 04/10] Feature: Allow adding delete protection for VMs & volumes (#9633) Co-authored-by: Suresh Kumar Anaparti --- .../main/java/com/cloud/storage/Volume.java | 10 +++-- .../com/cloud/storage/VolumeApiService.java | 4 +- .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/command/user/vm/UpdateVMCmd.java | 12 ++++++ .../command/user/volume/UpdateVolumeCmd.java | 14 ++++++- .../api/response/UserVmResponse.java | 12 ++++++ .../api/response/VolumeResponse.java | 12 ++++++ .../main/java/com/cloud/storage/VolumeVO.java | 12 ++++++ .../main/java/com/cloud/vm/VMInstanceVO.java | 14 +++++-- .../main/java/com/cloud/vm/dao/UserVmDao.java | 6 ++- .../java/com/cloud/vm/dao/UserVmDaoImpl.java | 8 +++- .../META-INF/db/schema-41910to42000.sql | 3 ++ .../META-INF/db/views/cloud.user_vm_view.sql | 1 + .../META-INF/db/views/cloud.volume_view.sql | 1 + .../storage/volume/VolumeObject.java | 5 +++ .../api/query/dao/UserVmJoinDaoImpl.java | 6 +++ .../api/query/dao/VolumeJoinDaoImpl.java | 6 +++ .../com/cloud/api/query/vo/UserVmJoinVO.java | 6 +++ .../com/cloud/api/query/vo/VolumeJoinVO.java | 7 ++++ .../cloud/storage/VolumeApiServiceImpl.java | 16 +++++++- .../main/java/com/cloud/vm/UserVmManager.java | 10 ++++- .../java/com/cloud/vm/UserVmManagerImpl.java | 37 ++++++++++++++++--- .../com/cloud/vm/UserVmManagerImplTest.java | 4 +- test/integration/smoke/test_vm_life_cycle.py | 28 ++++++++++++++ test/integration/smoke/test_volumes.py | 27 ++++++++++++++ tools/marvin/marvin/lib/base.py | 8 ++++ ui/public/locales/en.json | 1 + ui/src/config/section/compute.js | 3 +- ui/src/config/section/storage.js | 4 +- ui/src/views/compute/EditVM.vue | 11 ++++++ 30 files changed, 261 insertions(+), 28 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/Volume.java b/api/src/main/java/com/cloud/storage/Volume.java index 40c5660b2df..c7fbdb0a544 100644 --- a/api/src/main/java/com/cloud/storage/Volume.java +++ b/api/src/main/java/com/cloud/storage/Volume.java @@ -271,11 +271,13 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba void setExternalUuid(String externalUuid); - public Long getPassphraseId(); + Long getPassphraseId(); - public void setPassphraseId(Long id); + void setPassphraseId(Long id); - public String getEncryptFormat(); + String getEncryptFormat(); - public void setEncryptFormat(String encryptFormat); + void setEncryptFormat(String encryptFormat); + + boolean isDeleteProtection(); } diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java index f9cba14679e..6f4c7aa09e2 100644 --- a/api/src/main/java/com/cloud/storage/VolumeApiService.java +++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java @@ -117,7 +117,9 @@ public interface VolumeApiService { Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List zoneIds) throws ResourceAllocationException; - Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, String customId, long owner, String chainInfo, String name); + Volume updateVolume(long volumeId, String path, String state, Long storageId, + Boolean displayVolume, Boolean deleteProtection, + String customId, long owner, String chainInfo, String name); /** * Extracts the volume to a particular location. diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index ad953e97867..bb16b0ff90d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -138,6 +138,7 @@ public class ApiConstants { public static final String DATACENTER_NAME = "datacentername"; public static final String DATADISK_OFFERING_LIST = "datadiskofferinglist"; public static final String DEFAULT_VALUE = "defaultvalue"; + public static final String DELETE_PROTECTION = "deleteprotection"; public static final String DESCRIPTION = "description"; public static final String DESTINATION = "destination"; public static final String DESTINATION_ZONE_ID = "destzoneid"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index e5d625fe70d..0f5dade96d2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -146,6 +146,14 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction, @Parameter(name = ApiConstants.EXTRA_CONFIG, type = CommandType.STRING, since = "4.12", description = "an optional URL encoded string that can be passed to the virtual machine upon successful deployment", length = 5120) private String extraConfig; + @Parameter(name = ApiConstants.DELETE_PROTECTION, + type = CommandType.BOOLEAN, since = "4.20.0", + description = "Set delete protection for the virtual machine. If " + + "true, the instance will be protected from deletion. " + + "Note: If the instance is managed by another service like" + + " autoscaling groups or CKS, delete protection will be ignored.") + private Boolean deleteProtection; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -215,6 +223,10 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction, return cleanupDetails == null ? false : cleanupDetails.booleanValue(); } + public Boolean getDeleteProtection() { + return deleteProtection; + } + public Map> getDhcpOptionsMap() { Map> dhcpOptionsMap = new HashMap<>(); if (dhcpOptionsNetworkList != null && !dhcpOptionsNetworkList.isEmpty()) { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java index 467c587cc73..22b819c8cba 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java @@ -77,6 +77,14 @@ public class UpdateVolumeCmd extends BaseAsyncCustomIdCmd implements UserCmd { @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "new name of the volume", since = "4.16") private String name; + @Parameter(name = ApiConstants.DELETE_PROTECTION, + type = CommandType.BOOLEAN, since = "4.20.0", + description = "Set delete protection for the volume. If true, The volume " + + "will be protected from deletion. Note: If the volume is managed by " + + "another service like autoscaling groups or CKS, delete protection will be " + + "ignored.") + private Boolean deleteProtection; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -109,6 +117,10 @@ public class UpdateVolumeCmd extends BaseAsyncCustomIdCmd implements UserCmd { return name; } + public Boolean getDeleteProtection() { + return deleteProtection; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -168,7 +180,7 @@ public class UpdateVolumeCmd extends BaseAsyncCustomIdCmd implements UserCmd { public void execute() { CallContext.current().setEventDetails("Volume Id: " + this._uuidMgr.getUuid(Volume.class, getId())); Volume result = _volumeService.updateVolume(getId(), getPath(), getState(), getStorageId(), getDisplayVolume(), - getCustomId(), getEntityOwnerId(), getChainInfo(), getName()); + getDeleteProtection(), getCustomId(), getEntityOwnerId(), getChainInfo(), getName()); if (result != null) { VolumeResponse response = _responseGenerator.createVolumeResponse(getResponseView(), result); response.setResponseName(getCommandName()); diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java index df9a474213b..1f4b493fba2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java @@ -320,6 +320,10 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co @Param(description = "true if vm contains XS/VMWare tools inorder to support dynamic scaling of VM cpu/memory.") private Boolean isDynamicallyScalable; + @SerializedName(ApiConstants.DELETE_PROTECTION) + @Param(description = "true if vm has delete protection.", since = "4.20.0") + private boolean deleteProtection; + @SerializedName(ApiConstants.SERVICE_STATE) @Param(description = "State of the Service from LB rule") private String serviceState; @@ -995,6 +999,14 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co isDynamicallyScalable = dynamicallyScalable; } + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } + public String getOsTypeId() { return osTypeId; } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java index 4ac17e9832b..209ca57c50d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java @@ -261,6 +261,10 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co @Param(description = "true if storage snapshot is supported for the volume, false otherwise", since = "4.16") private boolean supportsStorageSnapshot; + @SerializedName(ApiConstants.DELETE_PROTECTION) + @Param(description = "true if volume has delete protection.", since = "4.20.0") + private boolean deleteProtection; + @SerializedName(ApiConstants.PHYSICAL_SIZE) @Param(description = "the bytes actually consumed on disk") private Long physicalsize; @@ -584,6 +588,14 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co return this.supportsStorageSnapshot; } + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } + public String getIsoId() { return isoId; } diff --git a/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java b/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java index e12859ea8d6..c105acf40b8 100644 --- a/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/VolumeVO.java @@ -182,6 +182,9 @@ public class VolumeVO implements Volume { @Column(name = "encrypt_format") private String encryptFormat; + @Column(name = "delete_protection") + private boolean deleteProtection; + // Real Constructor public VolumeVO(Type type, String name, long dcId, long domainId, @@ -678,4 +681,13 @@ public class VolumeVO implements Volume { public String getEncryptFormat() { return encryptFormat; } public void setEncryptFormat(String encryptFormat) { this.encryptFormat = encryptFormat; } + + @Override + public boolean isDeleteProtection() { + return deleteProtection; + } + + public void setDeleteProtection(boolean deleteProtection) { + this.deleteProtection = deleteProtection; + } } diff --git a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java index a1600e04350..a1d9f4a8089 100644 --- a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java +++ b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java @@ -167,10 +167,8 @@ public class VMInstanceVO implements VirtualMachine, FiniteStateObject details; @@ -542,6 +540,14 @@ public class VMInstanceVO implements VirtualMachine, FiniteStateObject getEntityType() { return VirtualMachine.class; diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java index 39c65866658..7de543e69d3 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java @@ -53,7 +53,11 @@ public interface UserVmDao extends GenericDao { * @param hostName TODO * @param instanceName */ - void updateVM(long id, String displayName, boolean enable, Long osTypeId, String userData, Long userDataId, String userDataDetails, boolean displayVm, boolean isDynamicallyScalable, String customId, String hostName, String instanceName); + void updateVM(long id, String displayName, boolean enable, Long osTypeId, + String userData, Long userDataId, String userDataDetails, + boolean displayVm, boolean isDynamicallyScalable, + boolean deleteProtection, String customId, String hostName, + String instanceName); List findDestroyedVms(Date date); diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java index 536779125e2..cc8b9fc59a8 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java @@ -274,8 +274,11 @@ public class UserVmDaoImpl extends GenericDaoBase implements Use } @Override - public void updateVM(long id, String displayName, boolean enable, Long osTypeId, String userData, Long userDataId, String userDataDetails, boolean displayVm, - boolean isDynamicallyScalable, String customId, String hostName, String instanceName) { + public void updateVM(long id, String displayName, boolean enable, Long osTypeId, + String userData, Long userDataId, String userDataDetails, + boolean displayVm, boolean isDynamicallyScalable, + boolean deleteProtection, String customId, String hostName, + String instanceName) { UserVmVO vo = createForUpdate(); vo.setDisplayName(displayName); vo.setHaEnabled(enable); @@ -285,6 +288,7 @@ public class UserVmDaoImpl extends GenericDaoBase implements Use vo.setUserDataDetails(userDataDetails); vo.setDisplayVm(displayVm); vo.setDynamicallyScalable(isDynamicallyScalable); + vo.setDeleteProtection(deleteProtection); if (hostName != null) { vo.setHostName(hostName); } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql b/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql index ac7d7936833..8f0076ad46b 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql @@ -620,3 +620,6 @@ INSERT IGNORE INTO `cloud`.`hypervisor_capabilities` (uuid, hypervisor_type, hyp INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) SELECT UUID(),'VMware', '8.0.2', guest_os_name, guest_os_id, utc_timestamp(), 0 FROM `cloud`.`guest_os_hypervisor` WHERE hypervisor_type='VMware' AND hypervisor_version='8.0'; INSERT IGNORE INTO `cloud`.`hypervisor_capabilities` (uuid, hypervisor_type, hypervisor_version, max_guests_limit, security_group_enabled, max_data_volumes_limit, max_hosts_per_cluster, storage_motion_supported, vm_snapshot_enabled) values (UUID(), 'VMware', '8.0.3', 1024, 0, 59, 64, 1, 1); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) SELECT UUID(),'VMware', '8.0.3', guest_os_name, guest_os_id, utc_timestamp(), 0 FROM `cloud`.`guest_os_hypervisor` WHERE hypervisor_type='VMware' AND hypervisor_version='8.0'; + +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vm_instance', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for vm" '); +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'delete_protection', 'boolean DEFAULT FALSE COMMENT "delete protection for volumes" '); diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql index de48272006d..97cb7b735cf 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql @@ -54,6 +54,7 @@ SELECT `vm_instance`.`instance_name` AS `instance_name`, `vm_instance`.`guest_os_id` AS `guest_os_id`, `vm_instance`.`display_vm` AS `display_vm`, + `vm_instance`.`delete_protection` AS `delete_protection`, `guest_os`.`uuid` AS `guest_os_uuid`, `vm_instance`.`pod_id` AS `pod_id`, `host_pod_ref`.`uuid` AS `pod_uuid`, diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql index 950dcddf4c7..ffeb93e8fa7 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql @@ -40,6 +40,7 @@ SELECT `volumes`.`chain_info` AS `chain_info`, `volumes`.`external_uuid` AS `external_uuid`, `volumes`.`encrypt_format` AS `encrypt_format`, + `volumes`.`delete_protection` AS `delete_protection`, `account`.`id` AS `account_id`, `account`.`uuid` AS `account_uuid`, `account`.`account_name` AS `account_name`, diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 1b3bec0e907..825a8cbd941 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -935,6 +935,11 @@ public class VolumeObject implements VolumeInfo { volumeVO.setEncryptFormat(encryptFormat); } + @Override + public boolean isDeleteProtection() { + return volumeVO.isDeleteProtection(); + } + @Override public boolean isFollowRedirects() { return followRedirects; diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index bd97178e8a8..af26a242db4 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -426,6 +426,12 @@ public class UserVmJoinDaoImpl extends GenericDaoBaseWithTagInformation getEntityType() { diff --git a/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java b/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java index 79f558a3ef5..2ae720fa852 100644 --- a/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java +++ b/server/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java @@ -280,6 +280,9 @@ public class VolumeJoinVO extends BaseViewWithTagInformationVO implements Contro @Column(name = "encrypt_format") private String encryptionFormat = null; + @Column(name = "delete_protection") + protected Boolean deleteProtection; + public VolumeJoinVO() { } @@ -619,6 +622,10 @@ public class VolumeJoinVO extends BaseViewWithTagInformationVO implements Contro return encryptionFormat; } + public Boolean getDeleteProtection() { + return deleteProtection; + } + @Override public Class getEntityType() { return Volume.class; diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index a392cecbeb4..0f215874c3a 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1699,6 +1699,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } public void validateDestroyVolume(Volume volume, Account caller, boolean expunge, boolean forceExpunge) { + if (volume.isDeleteProtection()) { + throw new InvalidParameterValueException(String.format( + "Volume [id = %s, name = %s] has delete protection enabled and cannot be deleted.", + volume.getUuid(), volume.getName())); + } + if (expunge) { // When trying to expunge, permission is denied when the caller is not an admin and the AllowUserExpungeRecoverVolume is false for the caller. final Long userId = caller.getAccountId(); @@ -2757,13 +2763,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Override @ActionEvent(eventType = EventTypes.EVENT_VOLUME_UPDATE, eventDescription = "updating volume", async = true) - public Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, + public Volume updateVolume(long volumeId, String path, String state, Long storageId, + Boolean displayVolume, Boolean deleteProtection, String customId, long entityOwnerId, String chainInfo, String name) { Account caller = CallContext.current().getCallingAccount(); if (!_accountMgr.isRootAdmin(caller.getId())) { if (path != null || state != null || storageId != null || displayVolume != null || customId != null || chainInfo != null) { - throw new InvalidParameterValueException("The domain admin and normal user are not allowed to update volume except volume name"); + throw new InvalidParameterValueException("The domain admin and normal user are " + + "not allowed to update volume except volume name & delete protection"); } } @@ -2815,6 +2823,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic volume.setName(name); } + if (deleteProtection != null) { + volume.setDeleteProtection(deleteProtection); + } + updateDisplay(volume, displayVolume); _volsDao.update(volumeId, volume); diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java index 31b0bc40597..f2a8a672d42 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManager.java +++ b/server/src/main/java/com/cloud/vm/UserVmManager.java @@ -141,8 +141,14 @@ public interface UserVmManager extends UserVmService { boolean setupVmForPvlan(boolean add, Long hostId, NicProfile nic); - UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, Boolean isDisplayVmEnabled, Long osTypeId, String userData, - Long userDataId, String userDataDetails, Boolean isDynamicallyScalable, HTTPMethod httpMethod, String customId, String hostName, String instanceName, List securityGroupIdList, Map> extraDhcpOptionsMap) throws ResourceUnavailableException, InsufficientCapacityException; + UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, + Boolean isDisplayVmEnabled, Boolean deleteProtection, + Long osTypeId, String userData, Long userDataId, + String userDataDetails, Boolean isDynamicallyScalable, + HTTPMethod httpMethod, String customId, String hostName, + String instanceName, List securityGroupIdList, + Map> extraDhcpOptionsMap + ) throws ResourceUnavailableException, InsufficientCapacityException; //the validateCustomParameters, save and remove CustomOfferingDetils functions can be removed from the interface once we can //find a common place for all the scaling and upgrading code of both user and systemvms. diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 17596163c37..cf81a78070f 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2921,8 +2921,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } } - return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, osTypeId, userData, userDataId, userDataDetails, isDynamicallyScalable, - cmd.getHttpMethod(), cmd.getCustomId(), hostName, cmd.getInstanceName(), securityGroupIdList, cmd.getDhcpOptionsMap()); + return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, + cmd.getDeleteProtection(), osTypeId, userData, + userDataId, userDataDetails, isDynamicallyScalable, cmd.getHttpMethod(), + cmd.getCustomId(), hostName, cmd.getInstanceName(), securityGroupIdList, + cmd.getDhcpOptionsMap()); } private boolean isExtraConfig(String detailName) { @@ -3023,9 +3026,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } @Override - public UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, Boolean isDisplayVmEnabled, Long osTypeId, String userData, - Long userDataId, String userDataDetails, Boolean isDynamicallyScalable, HTTPMethod httpMethod, String customId, String hostName, String instanceName, List securityGroupIdList, Map> extraDhcpOptionsMap) - throws ResourceUnavailableException, InsufficientCapacityException { + public UserVm updateVirtualMachine(long id, String displayName, String group, Boolean ha, + Boolean isDisplayVmEnabled, Boolean deleteProtection, + Long osTypeId, String userData, Long userDataId, + String userDataDetails, Boolean isDynamicallyScalable, + HTTPMethod httpMethod, String customId, String hostName, + String instanceName, List securityGroupIdList, + Map> extraDhcpOptionsMap + ) throws ResourceUnavailableException, InsufficientCapacityException { UserVmVO vm = _vmDao.findById(id); if (vm == null) { throw new CloudRuntimeException("Unable to find virtual machine with id " + id); @@ -3060,6 +3068,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir isDisplayVmEnabled = vm.isDisplayVm(); } + if (deleteProtection == null) { + deleteProtection = vm.isDeleteProtection(); + } + boolean updateUserdata = false; if (userData != null) { // check and replace newlines @@ -3174,7 +3186,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir .getUuid(), nic.getId(), extraDhcpOptionsMap); } - _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId, userDataDetails, isDisplayVmEnabled, isDynamicallyScalable, customId, hostName, instanceName); + _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId, + userDataDetails, isDisplayVmEnabled, isDynamicallyScalable, + deleteProtection, customId, hostName, instanceName); if (updateUserdata) { updateUserData(vm); @@ -3411,6 +3425,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return vm; } + if (vm.isDeleteProtection()) { + throw new InvalidParameterValueException(String.format( + "Instance [id = %s, name = %s] has delete protection enabled and cannot be deleted.", + vm.getUuid(), vm.getName())); + } + // check if vm belongs to AutoScale vm group in Disabled state autoScaleManager.checkIfVmActionAllowed(vmId); @@ -8586,6 +8606,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (!(volume.getVolumeType() == Volume.Type.ROOT || volume.getVolumeType() == Volume.Type.DATADISK)) { throw new InvalidParameterValueException("Please specify volume of type " + Volume.Type.DATADISK.toString() + " or " + Volume.Type.ROOT.toString()); } + if (volume.isDeleteProtection()) { + throw new InvalidParameterValueException(String.format( + "Volume [id = %s, name = %s] has delete protection enabled and cannot be deleted", + volume.getUuid(), volume.getName())); + } } } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 90d857b5816..8316c57d67d 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -487,7 +487,7 @@ public class UserVmManagerImplTest { Mockito.verify(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand); Mockito.verify(userVmManagerImpl).updateVirtualMachine(nullable(Long.class), nullable(String.class), nullable(String.class), nullable(Boolean.class), - nullable(Boolean.class), nullable(Long.class), + nullable(Boolean.class), nullable(Boolean.class), nullable(Long.class), nullable(String.class), nullable(Long.class), nullable(String.class), nullable(Boolean.class), nullable(HTTPMethod.class), nullable(String.class), nullable(String.class), nullable(String.class), nullable(List.class), nullable(Map.class)); @@ -498,7 +498,7 @@ public class UserVmManagerImplTest { Mockito.doNothing().when(userVmManagerImpl).validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand); Mockito.doReturn(new ArrayList()).when(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand); Mockito.lenient().doReturn(Mockito.mock(UserVm.class)).when(userVmManagerImpl).updateVirtualMachine(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), - Mockito.anyBoolean(), Mockito.anyLong(), + Mockito.anyBoolean(), Mockito.anyBoolean(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.any(HTTPMethod.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyList(), Mockito.anyMap()); } diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index c05ae2ad42e..c7c9a01bd32 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -1035,6 +1035,34 @@ class TestVMLifeCycle(cloudstackTestCase): return + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") + def test_14_destroy_vm_delete_protection(self): + """Test destroy Virtual Machine with delete protection + """ + + # Validate the following + # 1. Should not be able to delete the VM when delete protection is enabled + # 2. Should be able to delete the VM after disabling delete protection + + vm = VirtualMachine.create( + self.apiclient, + self.services["small"], + serviceofferingid=self.small_offering.id, + mode=self.services["mode"], + startvm=False + ) + + vm.update(self.apiclient, deleteprotection=True) + try: + vm.delete(self.apiclient) + self.fail("VM shouldn't get deleted with delete protection enabled") + except Exception as e: + self.debug("Expected exception: %s" % e) + + vm.update(self.apiclient, deleteprotection=False) + vm.delete(self.apiclient) + + return class TestSecuredVmMigration(cloudstackTestCase): diff --git a/test/integration/smoke/test_volumes.py b/test/integration/smoke/test_volumes.py index 7d64a27eaf2..28a029adf70 100644 --- a/test/integration/smoke/test_volumes.py +++ b/test/integration/smoke/test_volumes.py @@ -1038,6 +1038,33 @@ class TestVolumes(cloudstackTestCase): ) return + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_14_delete_volume_delete_protection(self): + """Delete a Volume with delete protection + + # Validate the following + # 1. delete volume will fail when delete protection is enabled + # 2. delete volume is successful when delete protection is disabled + """ + + volume = Volume.create( + self.apiclient, + self.services, + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering.id + ) + volume.update(self.apiclient, deleteprotection=True) + try: + volume.delete(self.apiclient) + self.fail("Volume delete should have failed with delete protection enabled") + except Exception as e: + self.debug("Volume delete failed as expected with error: %s" % e) + + volume.update(self.apiclient, deleteprotection=False) + volume.destroy(self.apiclient, expunge=True) + class TestVolumeEncryption(cloudstackTestCase): diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index e0a57c39924..557434ea2ee 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1160,6 +1160,14 @@ class Volume: return Volume(apiclient.createVolume(cmd).__dict__) + def update(self, apiclient, **kwargs): + """Updates the volume""" + + cmd = updateVolume.updateVolumeCmd() + cmd.id = self.id + [setattr(cmd, k, v) for k, v in list(kwargs.items())] + return (apiclient.updateVolume(cmd)) + @classmethod def create_custom_disk(cls, apiclient, services, account=None, domainid=None, diskofferingid=None, projectid=None): diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index b011ddaf6eb..6ca710d4095 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -740,6 +740,7 @@ "label.deleting.iso": "Deleting ISO", "label.deleting.snapshot": "Deleting Snapshot", "label.deleting.template": "Deleting Template", +"label.deleteprotection": "Delete protection", "label.demote.project.owner": "Demote Account to regular role", "label.demote.project.owner.user": "Demote User to regular role", "label.deny": "Deny", diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index 51bbbb1cff4..6d7875368f5 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -81,7 +81,8 @@ export default { details: () => { var fields = ['name', 'displayname', 'id', 'state', 'ipaddress', 'ip6address', 'templatename', 'ostypename', 'serviceofferingname', 'isdynamicallyscalable', 'haenable', 'hypervisor', 'boottype', 'bootmode', 'account', - 'domain', 'zonename', 'userdataid', 'userdataname', 'userdataparams', 'userdatadetails', 'userdatapolicy', 'hostcontrolstate'] + 'domain', 'zonename', 'userdataid', 'userdataname', 'userdataparams', 'userdatadetails', 'userdatapolicy', + 'hostcontrolstate', 'deleteprotection'] const listZoneHaveSGEnabled = store.getters.zones.filter(zone => zone.securitygroupsenabled === true) if (!listZoneHaveSGEnabled || listZoneHaveSGEnabled.length === 0) { return fields diff --git a/ui/src/config/section/storage.js b/ui/src/config/section/storage.js index 66c6cd4d0d1..d979e7395e0 100644 --- a/ui/src/config/section/storage.js +++ b/ui/src/config/section/storage.js @@ -62,7 +62,7 @@ export default { return fields }, - details: ['name', 'id', 'type', 'storagetype', 'diskofferingdisplaytext', 'deviceid', 'sizegb', 'physicalsize', 'provisioningtype', 'utilization', 'diskkbsread', 'diskkbswrite', 'diskioread', 'diskiowrite', 'diskiopstotal', 'miniops', 'maxiops', 'path'], + details: ['name', 'id', 'type', 'storagetype', 'diskofferingdisplaytext', 'deviceid', 'sizegb', 'physicalsize', 'provisioningtype', 'utilization', 'diskkbsread', 'diskkbswrite', 'diskioread', 'diskiowrite', 'diskiopstotal', 'miniops', 'maxiops', 'path', 'deleteprotection'], related: [{ name: 'snapshot', title: 'label.snapshots', @@ -148,7 +148,7 @@ export default { icon: 'edit-outlined', label: 'label.edit', dataView: true, - args: ['name'], + args: ['name', 'deleteprotection'], mapping: { account: { value: (record) => { return record.account } diff --git a/ui/src/views/compute/EditVM.vue b/ui/src/views/compute/EditVM.vue index 550c4645ed6..87e6d96d6c6 100644 --- a/ui/src/views/compute/EditVM.vue +++ b/ui/src/views/compute/EditVM.vue @@ -111,6 +111,13 @@ + + + + +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -175,6 +182,7 @@ export default { displayname: this.resource.displayname, ostypeid: this.resource.ostypeid, isdynamicallyscalable: this.resource.isdynamicallyscalable, + deleteprotection: this.resource.deleteprotection, group: this.resource.group, securitygroupids: this.resource.securitygroup.map(x => x.id), userdata: '', @@ -314,6 +322,9 @@ export default { if (values.isdynamicallyscalable !== undefined) { params.isdynamicallyscalable = values.isdynamicallyscalable } + if (values.deleteprotection !== undefined) { + params.deleteprotection = values.deleteprotection + } if (values.haenable !== undefined) { params.haenable = values.haenable } From b068c68bffb4bc34b5a91ce1ba995164848f5e67 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 9 Sep 2024 15:51:43 +0200 Subject: [PATCH 05/10] merge conflict (in loggers) --- .../kvm/storage/LinstorStorageAdaptor.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index 13bd89bd380..fabe2ab3536 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -154,8 +154,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map details) { - logger.debug(String.format( - "Linstor createStoragePool: name: '%s', host: '%s', path: %s, userinfo: %s", name, host, path, userInfo)); + logger.debug("Linstor createStoragePool: name: '{}', host: '{}', path: {}, userinfo: {}", name, host, path, userInfo); LinstorStoragePool storagePool = new LinstorStoragePool(name, host, port, userInfo, type, this); MapStorageUuidToStoragePool.put(name, storagePool); @@ -190,7 +189,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, QemuImg.PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, byte[] passphrase) { - logger.debug(String.format("Linstor.createPhysicalDisk: %s;%s", name, format)); + logger.debug("Linstor.createPhysicalDisk: {};{}", name, format); final String rscName = getLinstorRscName(name); LinstorStoragePool lpool = (LinstorStoragePool) pool; final DevelopersApi api = getLinstorAPI(pool); @@ -231,7 +230,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { throw new CloudRuntimeException("Linstor: viewResources didn't return resources or volumes."); } } catch (ApiException apiEx) { - logger.error(String.format("Linstor.createPhysicalDisk: ApiException: %s", apiEx.getBestMessage())); + logger.error("Linstor.createPhysicalDisk: ApiException: {}", apiEx.getBestMessage()); throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx); } } @@ -254,9 +253,8 @@ public class LinstorStorageAdaptor implements StorageAdaptor { rcm.setOverrideProps(props); ApiCallRcList answers = api.resourceConnectionModify(rscName, inUseNode, localNodeName, rcm); if (answers.hasError()) { - logger.error(String.format( - "Unable to set protocol C and 'allow-two-primaries' on %s/%s/%s", - inUseNode, localNodeName, rscName)); + logger.error("Unable to set protocol C and 'allow-two-primaries' on {}/{}/{}", + inUseNode, localNodeName, rscName); // do not fail here as adding allow-two-primaries property is only a problem while live migrating } } @@ -265,7 +263,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { @Override public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details) { - logger.debug(String.format("Linstor: connectPhysicalDisk %s:%s -> %s", pool.getUuid(), volumePath, details)); + logger.debug("Linstor: connectPhysicalDisk {}:{} -> {}", pool.getUuid(), volumePath, details); if (volumePath == null) { logger.warn("volumePath is null, ignoring"); return false; @@ -304,11 +302,10 @@ public class LinstorStorageAdaptor implements StorageAdaptor { rcm.deleteProps(deleteProps); ApiCallRcList answers = api.resourceConnectionModify(rscName, localNodeName, inUseNode, rcm); if (answers.hasError()) { - logger.error( - String.format("Failed to remove 'protocol' and 'allow-two-primaries' on %s/%s/%s: %s", + logger.error("Failed to remove 'protocol' and 'allow-two-primaries' on {}/{}/{}: {}", localNodeName, inUseNode, - rscName, LinstorUtil.getBestErrorMessage(answers))); + rscName, LinstorUtil.getBestErrorMessage(answers)); // do not fail here as removing allow-two-primaries property isn't fatal } } @@ -512,7 +509,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { @Override public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMStoragePool destPools, int timeout, byte[] srcPassphrase, byte[] destPassphrase, Storage.ProvisioningType provisioningType) { - logger.debug(String.format("Linstor.copyPhysicalDisk: %s -> %s", disk.getPath(), name)); + logger.debug("Linstor.copyPhysicalDisk: {} -> {}", disk.getPath(), name); final QemuImg.PhysicalDiskFormat sourceFormat = disk.getFormat(); final String sourcePath = disk.getPath(); @@ -526,11 +523,11 @@ public class LinstorStorageAdaptor implements StorageAdaptor { try { LinstorUtil.applyAuxProps(api, rscName, disk.getDispName(), disk.getVmName()); } catch (ApiException apiExc) { - s_logger.error(String.format("Error setting aux properties for %s", rscName)); + logger.error("Error setting aux properties for {}", rscName); logLinstorAnswers(apiExc.getApiCallRcList()); } - logger.debug(String.format("Linstor.copyPhysicalDisk: dstPath: %s", dstDisk.getPath())); + logger.debug("Linstor.copyPhysicalDisk: dstPath: {}", dstDisk.getPath()); final QemuImgFile destFile = new QemuImgFile(dstDisk.getPath()); destFile.setFormat(dstDisk.getFormat()); destFile.setSize(disk.getVirtualSize()); From 501d8c1e09173717a4ed98b43acf41cdcd245f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Mon, 9 Sep 2024 15:39:42 -0300 Subject: [PATCH 06/10] Add logs to CPVM connection process (#8924) * increment cpvm connection logs * remove sourceIp variable * increment cpvm connection logs * extract duplicate error messages to variables * change logs level from trace to debug in authenticateToVNCServer * add logs in trace level inside of connection loop * remove redundant trace log * add logs to ConsoleProxyNoVNCHandler class * retrieve client source IP * add periods to log messages * change log levels from warn to error inside of catch blocks * add client IP to successful authentication log * replace concatenation with String.format() * remove String.format() and use log4j2 new features instead * remove String.format() and use log4j2 new features instead * apply Daan's suggestion Co-authored-by: dahn * resolve conflicts * fix logs with three parameters * get correct client IP * use log4j dependencies directly * apply winterhazel's suggestion Co-authored-by: Fabricio Duarte * remove log proxy * address winterhazel's suggestions on ConsoleProxyNoVncClient class * address winterhazel's suggestions on ConsoleProxyNoVNCHandler class * address winterhazel's suggestions on ConsoleProxyNoVNCHandler class Co-authored-by: Fabricio Duarte --------- Co-authored-by: dahn Co-authored-by: Fabricio Duarte --- .../servlet/ConsoleProxyClientParam.java | 17 ++ .../consoleproxy/AjaxFIFOImageCache.java | 5 +- .../com/cloud/consoleproxy/ConsoleProxy.java | 8 +- .../consoleproxy/ConsoleProxyAjaxHandler.java | 5 +- .../ConsoleProxyAjaxImageHandler.java | 5 +- .../ConsoleProxyBaseServerFactoryImpl.java | 5 +- .../consoleproxy/ConsoleProxyClientParam.java | 16 ++ .../consoleproxy/ConsoleProxyCmdHandler.java | 5 +- .../ConsoleProxyHttpHandlerHelper.java | 5 +- .../ConsoleProxyLoggerFactory.java | 104 -------- .../consoleproxy/ConsoleProxyMonitor.java | 5 +- .../ConsoleProxyNoVNCHandler.java | 47 ++-- .../consoleproxy/ConsoleProxyNoVNCServer.java | 5 +- .../consoleproxy/ConsoleProxyNoVncClient.java | 38 +-- .../ConsoleProxyResourceHandler.java | 5 +- .../ConsoleProxyThumbnailHandler.java | 5 +- .../rdp/RdpBufferedImageCanvas.java | 6 +- .../com/cloud/consoleproxy/util/Logger.java | 223 ------------------ .../consoleproxy/util/LoggerFactory.java | 21 -- .../com/cloud/consoleproxy/util/RawHTTP.java | 5 +- .../consoleproxy/vnc/BufferedImageCanvas.java | 6 +- .../cloud/consoleproxy/vnc/NoVncClient.java | 21 +- .../com/cloud/consoleproxy/vnc/VncClient.java | 6 +- .../vnc/VncClientPacketSender.java | 6 +- .../vnc/VncServerPacketReceiver.java | 6 +- .../vnc/packet/server/AbstractRect.java | 5 +- .../vnc/packet/server/ServerCutText.java | 6 +- .../vnc/security/VncAuthSecurity.java | 6 +- .../vnc/security/VncTLSSecurity.java | 6 +- .../websocket/WebSocketReverseProxy.java | 6 +- 30 files changed, 172 insertions(+), 437 deletions(-) delete mode 100644 services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyLoggerFactory.java delete mode 100644 services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/Logger.java delete mode 100644 services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/LoggerFactory.java diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java index e23778c0b98..b416ab98288 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java @@ -34,7 +34,16 @@ public class ConsoleProxyClientParam { private String username; private String password; + /** + * IP that has generated the console endpoint + */ private String sourceIP; + + /** + * IP of the client that has connected to the console + */ + private String clientIp; + private String websocketUrl; private String sessionUuid; @@ -201,4 +210,12 @@ public class ConsoleProxyClientParam { public void setSessionUuid(String sessionUuid) { this.sessionUuid = sessionUuid; } + + public String getClientIp() { + return clientIp; + } + + public void setClientIp(String clientIp) { + this.clientIp = clientIp; + } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/AjaxFIFOImageCache.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/AjaxFIFOImageCache.java index 5a0a29977b5..1b94578d1e0 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/AjaxFIFOImageCache.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/AjaxFIFOImageCache.java @@ -21,10 +21,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class AjaxFIFOImageCache { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private List fifoQueue; private Map cache; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java index c841f76540d..22922f43f93 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java @@ -39,17 +39,19 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.core.config.Configurator; import org.eclipse.jetty.websocket.api.Session; -import com.cloud.consoleproxy.util.Logger; import com.cloud.utils.PropertiesUtil; import com.google.gson.Gson; import com.sun.net.httpserver.HttpServer; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + /** * * ConsoleProxy, singleton class that manages overall activities in console proxy process. To make legacy code work, we still */ public class ConsoleProxy { - protected static Logger LOGGER = Logger.getLogger(ConsoleProxy.class); + protected static Logger LOGGER = LogManager.getLogger(ConsoleProxy.class); public static final int KEYBOARD_RAW = 0; public static final int KEYBOARD_COOKED = 1; @@ -280,7 +282,6 @@ public class ConsoleProxy { public static void startWithContext(Properties conf, Object context, byte[] ksBits, String ksPassword, String password, Boolean isSourceIpCheckEnabled) { setEncryptorPassword(password); configLog4j(); - Logger.setFactory(new ConsoleProxyLoggerFactory()); LOGGER.info("Start console proxy with context"); if (conf != null) { @@ -427,7 +428,6 @@ public class ConsoleProxy { public static void main(String[] argv) { standaloneStart = true; configLog4j(); - Logger.setFactory(new ConsoleProxyLoggerFactory()); InputStream confs = ConsoleProxy.class.getResourceAsStream("/conf/consoleproxy.properties"); Properties conf = new Properties(); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxHandler.java index e42917db6aa..bfd25188c65 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxHandler.java @@ -32,10 +32,11 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyAjaxHandler implements HttpHandler { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); public ConsoleProxyAjaxHandler() { } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxImageHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxImageHandler.java index af200b0a0e8..bb5b9f6ec31 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxImageHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyAjaxImageHandler.java @@ -28,10 +28,11 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyAjaxImageHandler implements HttpHandler { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); @Override public void handle(HttpExchange t) throws IOException { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyBaseServerFactoryImpl.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyBaseServerFactoryImpl.java index b178f0d5d68..548b1a99261 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyBaseServerFactoryImpl.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyBaseServerFactoryImpl.java @@ -23,10 +23,11 @@ import javax.net.ssl.SSLServerSocket; import com.sun.net.httpserver.HttpServer; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyBaseServerFactoryImpl implements ConsoleProxyServerFactory { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); @Override public void init(byte[] ksBits, String ksPassword) { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java index aa1f2223a8c..01c4fa6480e 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java @@ -39,8 +39,16 @@ public class ConsoleProxyClientParam { private String password; private String websocketUrl; + /** + * IP that has generated the console endpoint + */ private String sourceIP; + /** + * IP of the client that has connected to the console + */ + private String clientIp; + private String sessionUuid; /** @@ -204,4 +212,12 @@ public class ConsoleProxyClientParam { public void setClientProvidedExtraSecurityToken(String clientProvidedExtraSecurityToken) { this.clientProvidedExtraSecurityToken = clientProvidedExtraSecurityToken; } + + public String getClientIp() { + return clientIp; + } + + public void setClientIp(String clientIp) { + this.clientIp = clientIp; + } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyCmdHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyCmdHandler.java index 400eb2b9984..606b4509512 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyCmdHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyCmdHandler.java @@ -24,10 +24,11 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyCmdHandler implements HttpHandler { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); @Override public void handle(HttpExchange t) throws IOException { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java index fb9d0794c22..48ac5f44ff2 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java @@ -19,10 +19,11 @@ package com.cloud.consoleproxy; import java.util.HashMap; import java.util.Map; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyHttpHandlerHelper { - protected static Logger LOGGER = Logger.getLogger(ConsoleProxyHttpHandlerHelper.class); + protected static Logger LOGGER = LogManager.getLogger(ConsoleProxyHttpHandlerHelper.class); public static Map getQueryMap(String query) { String[] params = query.split("&"); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyLoggerFactory.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyLoggerFactory.java deleted file mode 100644 index 74e393f64d8..00000000000 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyLoggerFactory.java +++ /dev/null @@ -1,104 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -package com.cloud.consoleproxy; - -import com.cloud.consoleproxy.util.Logger; -import com.cloud.consoleproxy.util.LoggerFactory; -import org.apache.logging.log4j.LogManager; - -public class ConsoleProxyLoggerFactory implements LoggerFactory { - public ConsoleProxyLoggerFactory() { - } - - @Override - public Logger getLogger(Class clazz) { - return new Log4jLogger(LogManager.getLogger(clazz)); - } - - public static class Log4jLogger extends Logger { - private org.apache.logging.log4j.Logger logger; - - public Log4jLogger(org.apache.logging.log4j.Logger logger) { - this.logger = logger; - } - - @Override - public boolean isTraceEnabled() { - return logger.isTraceEnabled(); - } - - @Override - public boolean isDebugEnabled() { - return logger.isDebugEnabled(); - } - - @Override - public boolean isInfoEnabled() { - return logger.isInfoEnabled(); - } - - @Override - public void trace(Object message) { - logger.trace(message); - } - - @Override - public void trace(Object message, Throwable exception) { - logger.trace(message, exception); - } - - @Override - public void info(Object message) { - logger.info(message); - } - - @Override - public void info(Object message, Throwable exception) { - logger.info(message, exception); - } - - @Override - public void debug(Object message) { - logger.debug(message); - } - - @Override - public void debug(Object message, Throwable exception) { - logger.debug(message, exception); - } - - @Override - public void warn(Object message) { - logger.warn(message); - } - - @Override - public void warn(Object message, Throwable exception) { - logger.warn(message, exception); - } - - @Override - public void error(Object message) { - logger.error(message); - } - - @Override - public void error(Object message, Throwable exception) { - logger.error(message, exception); - } - } -} diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyMonitor.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyMonitor.java index 378072ad804..3e224d8d4c4 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyMonitor.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyMonitor.java @@ -24,7 +24,8 @@ import java.util.HashMap; import java.util.Map; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.config.Configurator; // @@ -33,7 +34,7 @@ import org.apache.logging.log4j.core.config.Configurator; // itself and the shell script will re-launch console proxy // public class ConsoleProxyMonitor { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private String[] _argv; private Map _argMap = new HashMap(); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java index be0db7b8fb4..a9639d0b32e 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java @@ -23,7 +23,8 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.websocket.api.Session; @@ -40,7 +41,7 @@ import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory; public class ConsoleProxyNoVNCHandler extends WebSocketHandler { private ConsoleProxyNoVncClient viewer = null; - protected Logger logger = Logger.getLogger(ConsoleProxyNoVNCHandler.class); + protected Logger logger = LogManager.getLogger(getClass()); public ConsoleProxyNoVNCHandler() { super(); @@ -82,15 +83,16 @@ public class ConsoleProxyNoVNCHandler extends WebSocketHandler { String ticket = queryMap.get("ticket"); String displayName = queryMap.get("displayname"); String ajaxSessionIdStr = queryMap.get("sess"); - String console_url = queryMap.get("consoleurl"); - String console_host_session = queryMap.get("sessionref"); - String vm_locale = queryMap.get("locale"); + String consoleUrl = queryMap.get("consoleurl"); + String consoleHostSession = queryMap.get("sessionref"); + String vmLocale = queryMap.get("locale"); String hypervHost = queryMap.get("hypervHost"); String username = queryMap.get("username"); String password = queryMap.get("password"); String sourceIP = queryMap.get("sourceIP"); String websocketUrl = queryMap.get("websocketUrl"); String sessionUuid = queryMap.get("sessionUuid"); + String clientIp = session.getRemoteAddress().getAddress().getHostAddress(); if (tag == null) tag = ""; @@ -104,7 +106,7 @@ public class ConsoleProxyNoVNCHandler extends WebSocketHandler { try { port = Integer.parseInt(portStr); } catch (NumberFormatException e) { - logger.warn("Invalid number parameter in query string: " + portStr); + logger.error("Invalid port value in query string: {}. Expected a number.", portStr, e); throw new IllegalArgumentException(e); } @@ -112,12 +114,12 @@ public class ConsoleProxyNoVNCHandler extends WebSocketHandler { try { ajaxSessionId = Long.parseLong(ajaxSessionIdStr); } catch (NumberFormatException e) { - logger.warn("Invalid number parameter in query string: " + ajaxSessionIdStr); + logger.error("Invalid ajaxSessionId (sess) value in query string: {}. Expected a number.", ajaxSessionIdStr, e); throw new IllegalArgumentException(e); } } - if (! checkSessionSourceIp(session, sourceIP)) { + if (!checkSessionSourceIp(session, sourceIP, clientIp)) { return; } @@ -129,14 +131,17 @@ public class ConsoleProxyNoVNCHandler extends WebSocketHandler { param.setClientTag(tag); param.setTicket(ticket); param.setClientDisplayName(displayName); - param.setClientTunnelUrl(console_url); - param.setClientTunnelSession(console_host_session); - param.setLocale(vm_locale); + param.setClientTunnelUrl(consoleUrl); + param.setClientTunnelSession(consoleHostSession); + param.setLocale(vmLocale); param.setHypervHost(hypervHost); param.setUsername(username); param.setPassword(password); param.setWebsocketUrl(websocketUrl); param.setSessionUuid(sessionUuid); + param.setSourceIP(sourceIP); + param.setClientIp(clientIp); + if (queryMap.containsKey("extraSecurityToken")) { param.setExtraSecurityToken(queryMap.get("extraSecurityToken")); } @@ -144,8 +149,9 @@ public class ConsoleProxyNoVNCHandler extends WebSocketHandler { param.setClientProvidedExtraSecurityToken(queryMap.get("extra")); } viewer = ConsoleProxy.getNoVncViewer(param, ajaxSessionIdStr, session); + logger.info("Viewer has been created successfully [session UUID: {}, client IP: {}].", sessionUuid, clientIp); } catch (Exception e) { - logger.warn("Failed to create viewer due to " + e.getMessage(), e); + logger.error("Failed to create viewer [session UUID: {}, client IP: {}] due to {}.", sessionUuid, clientIp, e.getMessage(), e); return; } finally { if (viewer == null) { @@ -154,32 +160,35 @@ public class ConsoleProxyNoVNCHandler extends WebSocketHandler { } } - private boolean checkSessionSourceIp(final Session session, final String sourceIP) throws IOException { - // Verify source IP - String sessionSourceIP = session.getRemoteAddress().getAddress().getHostAddress(); - logger.info("Get websocket connection request from remote IP : " + sessionSourceIP); - if (ConsoleProxy.isSourceIpCheckEnabled && (sessionSourceIP == null || ! sessionSourceIP.equals(sourceIP))) { - logger.warn("Failed to access console as the source IP to request the console is " + sourceIP); + private boolean checkSessionSourceIp(final Session session, final String sourceIP, String sessionSourceIP) throws IOException { + logger.info("Verifying session source IP {} from WebSocket connection request.", sessionSourceIP); + if (ConsoleProxy.isSourceIpCheckEnabled && (sessionSourceIP == null || !sessionSourceIP.equals(sourceIP))) { + logger.warn("Failed to access console as the source IP to request the console is {}.", sourceIP); session.disconnect(); return false; } + logger.debug("Session source IP {} has been verified successfully.", sessionSourceIP); return true; } @OnWebSocketClose public void onClose(Session session, int statusCode, String reason) throws IOException, InterruptedException { + String sessionSourceIp = session.getRemoteAddress().getAddress().getHostAddress(); + logger.debug("Closing WebSocket session [source IP: {}, status code: {}].", sessionSourceIp, statusCode); if (viewer != null) { ConsoleProxy.removeViewer(viewer); } + logger.debug("WebSocket session [source IP: {}, status code: {}] closed successfully.", sessionSourceIp, statusCode); } @OnWebSocketFrame public void onFrame(Frame f) throws IOException { + logger.trace("Sending client [ID: {}] frame of {} bytes.", viewer.getClientId(), f.getPayloadLength()); viewer.sendClientFrame(f); } @OnWebSocketError public void onError(Throwable cause) { - logger.error("Error on websocket", cause); + logger.error("Error on WebSocket [client ID: {}, session UUID: {}].", cause, viewer.getClientId(), viewer.getSessionUuid()); } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCServer.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCServer.java index f65754169f6..3d94ed26b0b 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCServer.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCServer.java @@ -22,7 +22,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.security.KeyStore; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; @@ -34,7 +35,7 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; public class ConsoleProxyNoVNCServer { - protected static Logger LOGGER = Logger.getLogger(ConsoleProxyNoVNCServer.class); + protected static Logger LOGGER = LogManager.getLogger(ConsoleProxyNoVNCServer.class); public static final int WS_PORT = 8080; public static final int WSS_PORT = 8443; private static final String VNC_CONF_FILE_LOCATION = "/root/vncport"; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java index cf9e3cfddcc..fece5bfaa22 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java @@ -75,9 +75,9 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { @Override public boolean isFrontEndAlive() { - if (!connectionAlive || System.currentTimeMillis() - - getClientLastFrontEndActivityTime() > ConsoleProxy.VIEWER_LINGER_SECONDS * 1000) { - logger.info("Front end has been idle for too long"); + long unusedTime = System.currentTimeMillis() - getClientLastFrontEndActivityTime(); + if (!connectionAlive || unusedTime > ConsoleProxy.VIEWER_LINGER_SECONDS * 1000) { + logger.info("Front end has been idle for too long ({} ms).", unusedTime); return false; } return true; @@ -95,23 +95,24 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { client = new NoVncClient(); connectionAlive = true; this.sessionUuid = param.getSessionUuid(); + String clientSourceIp = param.getClientIp(); + logger.debug("Initializing client from IP {}.", clientSourceIp); updateFrontEndActivityTime(); Thread worker = new Thread(new Runnable() { public void run() { try { - String tunnelUrl = param.getClientTunnelUrl(); String tunnelSession = param.getClientTunnelSession(); String websocketUrl = param.getWebsocketUrl(); connectClientToVNCServer(tunnelUrl, tunnelSession, websocketUrl); - - authenticateToVNCServer(); + authenticateToVNCServer(clientSourceIp); int readBytes; byte[] b; while (connectionAlive) { + logger.trace("Connection with client [{}] [IP: {}] is alive.", clientId, clientSourceIp); if (client.isVncOverWebSocketConnection()) { if (client.isVncOverWebSocketConnectionOpen()) { updateFrontEndActivityTime(); @@ -122,7 +123,7 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { int nextBytes = client.getNextBytes(); bytesArr = new byte[nextBytes]; client.readBytes(bytesArr, nextBytes); - logger.trace(String.format("Read [%s] bytes from client [%s]", nextBytes, clientId)); + logger.trace("Read [{}] bytes from client [{}].", nextBytes, clientId); if (nextBytes > 0) { session.getRemote().sendBytes(ByteBuffer.wrap(bytesArr)); updateFrontEndActivityTime(); @@ -132,7 +133,7 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { } else { b = new byte[100]; readBytes = client.read(b); - logger.trace(String.format("Read [%s] bytes from client [%s]", readBytes, clientId)); + logger.trace("Read [{}] bytes from client [{}].", readBytes, clientId); if (readBytes == -1 || (readBytes > 0 && !sendReadBytesToNoVNC(b, readBytes))) { connectionAlive = false; } @@ -143,7 +144,7 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { logger.error("Error on sleep for vnc sessions", e); } } - logger.info(String.format("Connection with client [%s] is dead.", clientId)); + logger.info("Connection with client [{}] [IP: {}] is dead.", clientId, clientSourceIp); } catch (IOException e) { logger.error("Error on VNC client", e); } @@ -158,7 +159,7 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); updateFrontEndActivityTime(); } catch (WebSocketException | IOException e) { - logger.debug("Connection exception", e); + logger.error("VNC server connection exception.", e); return false; } return true; @@ -176,20 +177,24 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { * * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#7protocol-messages */ - private void authenticateToVNCServer() throws IOException { + private void authenticateToVNCServer(String clientSourceIp) throws IOException { if (client.isVncOverWebSocketConnection()) { + logger.debug("Authentication skipped for client [{}] [IP: {}] to VNC server due to WebSocket protocol usage.", clientId, clientSourceIp); return; } if (!client.isVncOverNioSocket()) { + logger.debug("Authenticating client [{}] [IP: {}] to VNC server.", clientId, clientSourceIp); String ver = client.handshake(); session.getRemote().sendBytes(ByteBuffer.wrap(ver.getBytes(), 0, ver.length())); byte[] b = client.authenticateTunnel(getClientHostPassword()); session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, 4)); } else { + logger.debug("Authenticating client [{}] [IP: {}] to VNC server through NIO Socket.", clientId, clientSourceIp); authenticateVNCServerThroughNioSocket(); } + logger.debug("Client [{}] [IP: {}] has been authenticated successfully to VNC server.", clientId, clientSourceIp); } /** @@ -233,9 +238,6 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { protected void authenticateVNCServerThroughNioSocket() { handshakePhase(); initialisationPhase(); - if (logger.isDebugEnabled()) { - logger.debug("Authenticated successfully"); - } } /** @@ -289,8 +291,7 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { logger.info(String.format("Connect to VNC over websocket URL: %s", websocketUrl)); ConsoleProxy.ensureRoute(NetUtils.extractHost(websocketUrl)); client.connectToWebSocket(websocketUrl, session); - } else if (tunnelUrl != null && !tunnelUrl.isEmpty() && tunnelSession != null - && !tunnelSession.isEmpty()) { + } else if (StringUtils.isNotBlank(tunnelUrl) && StringUtils.isNotBlank(tunnelSession)) { URI uri = new URI(tunnelUrl); logger.info(String.format("Connect to VNC server via tunnel. url: %s, session: %s", tunnelUrl, tunnelSession)); @@ -304,8 +305,10 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { ConsoleProxy.ensureRoute(getClientHostAddress()); client.connectTo(getClientHostAddress(), getClientHostPort()); } + + logger.info("Connection to VNC server has been established successfully."); } catch (Throwable e) { - logger.error("Unexpected exception", e); + logger.error("Unexpected exception while connecting to VNC server.", e); } } @@ -370,6 +373,7 @@ public class ConsoleProxyNoVncClient implements ConsoleProxyClient { } public void updateFrontEndActivityTime() { + logger.trace("Updating last front end activity time."); lastFrontEndActivityTime = System.currentTimeMillis(); } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyResourceHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyResourceHandler.java index 949e632786c..8764326b503 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyResourceHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyResourceHandler.java @@ -28,10 +28,11 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyResourceHandler implements HttpHandler { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); static Map s_mimeTypes; static { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyThumbnailHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyThumbnailHandler.java index 0103d9fa70e..e2ec7df5d69 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyThumbnailHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyThumbnailHandler.java @@ -32,10 +32,11 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ConsoleProxyThumbnailHandler implements HttpHandler { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); public ConsoleProxyThumbnailHandler() { } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/rdp/RdpBufferedImageCanvas.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/rdp/RdpBufferedImageCanvas.java index 7fd19a15d2f..3a40b79437b 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/rdp/RdpBufferedImageCanvas.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/rdp/RdpBufferedImageCanvas.java @@ -25,10 +25,12 @@ import java.util.List; import com.cloud.consoleproxy.ConsoleProxyRdpClient; import com.cloud.consoleproxy.util.ImageHelper; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.util.TileInfo; import com.cloud.consoleproxy.vnc.FrameBufferCanvas; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import common.BufferedImageCanvas; public class RdpBufferedImageCanvas extends BufferedImageCanvas implements FrameBufferCanvas { @@ -36,7 +38,7 @@ public class RdpBufferedImageCanvas extends BufferedImageCanvas implements Frame * */ private static final long serialVersionUID = 1L; - protected Logger logger = Logger.getLogger(RdpBufferedImageCanvas.class); + protected Logger logger = LogManager.getLogger(RdpBufferedImageCanvas.class); private final ConsoleProxyRdpClient _rdpClient; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/Logger.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/Logger.java deleted file mode 100644 index 042d64977c2..00000000000 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/Logger.java +++ /dev/null @@ -1,223 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -package com.cloud.consoleproxy.util; - -// logger facility for dynamic switch between console logger used in Applet and log4j based logger -public class Logger { - private static LoggerFactory factory = null; - - public static final int LEVEL_TRACE = 1; - public static final int LEVEL_DEBUG = 2; - public static final int LEVEL_INFO = 3; - public static final int LEVEL_WARN = 4; - public static final int LEVEL_ERROR = 5; - - private Class clazz; - private Logger logger; - - private static int level = LEVEL_INFO; - - public static Logger getLogger(Class clazz) { - return new Logger(clazz); - } - - public static void setFactory(LoggerFactory f) { - factory = f; - } - - public static void setLevel(int l) { - level = l; - } - - public Logger(Class clazz) { - this.clazz = clazz; - } - - protected Logger() { - } - - public boolean isTraceEnabled() { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - return logger.isTraceEnabled(); - } - return level <= LEVEL_TRACE; - } - - public boolean isDebugEnabled() { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - return logger.isDebugEnabled(); - } - return level <= LEVEL_DEBUG; - } - - public boolean isInfoEnabled() { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - return logger.isInfoEnabled(); - } - return level <= LEVEL_INFO; - } - - public void trace(Object message) { - - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.trace(message); - } else { - if (level <= LEVEL_TRACE) - System.out.println(message); - } - } - - public void trace(Object message, Throwable exception) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.trace(message, exception); - } else { - if (level <= LEVEL_TRACE) { - System.out.println(message); - if (exception != null) { - exception.printStackTrace(System.out); - } - } - } - } - - public void info(Object message) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.info(message); - } else { - if (level <= LEVEL_INFO) - System.out.println(message); - } - } - - public void info(Object message, Throwable exception) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.info(message, exception); - } else { - if (level <= LEVEL_INFO) { - System.out.println(message); - if (exception != null) { - exception.printStackTrace(System.out); - } - } - } - } - - public void debug(Object message) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.debug(message); - } else { - if (level <= LEVEL_DEBUG) - System.out.println(message); - } - } - - public void debug(Object message, Throwable exception) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.debug(message, exception); - } else { - if (level <= LEVEL_DEBUG) { - System.out.println(message); - if (exception != null) { - exception.printStackTrace(System.out); - } - } - } - } - - public void warn(Object message) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.warn(message); - } else { - if (level <= LEVEL_WARN) - System.out.println(message); - } - } - - public void warn(Object message, Throwable exception) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.warn(message, exception); - } else { - if (level <= LEVEL_WARN) { - System.out.println(message); - if (exception != null) { - exception.printStackTrace(System.out); - } - } - } - } - - public void error(Object message) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.error(message); - } else { - if (level <= LEVEL_ERROR) - System.out.println(message); - } - } - - public void error(Object message, Throwable exception) { - if (factory != null) { - if (logger == null) - logger = factory.getLogger(clazz); - - logger.error(message, exception); - } else { - if (level <= LEVEL_ERROR) { - System.out.println(message); - if (exception != null) { - exception.printStackTrace(System.out); - } - } - } - } -} diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/LoggerFactory.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/LoggerFactory.java deleted file mode 100644 index 121411adf16..00000000000 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/LoggerFactory.java +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -package com.cloud.consoleproxy.util; - -public interface LoggerFactory { - Logger getLogger(Class clazz); -} diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/RawHTTP.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/RawHTTP.java index bc47ca03d12..99945c8b4ca 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/RawHTTP.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/util/RawHTTP.java @@ -38,6 +38,9 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + // // This file is originally from XenConsole with modifications // @@ -48,7 +51,7 @@ import java.util.regex.Pattern; * connections and import/export operations. */ public final class RawHTTP { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private static final Pattern END_PATTERN = Pattern.compile("^\r\n$"); private static final Pattern HEADER_PATTERN = Pattern.compile("^([A-Z_a-z0-9-]+):\\s*(.*)\r\n$"); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/BufferedImageCanvas.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/BufferedImageCanvas.java index 9b86a8fbc66..0a168362ec1 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/BufferedImageCanvas.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/BufferedImageCanvas.java @@ -27,16 +27,18 @@ import java.io.IOException; import java.util.List; import com.cloud.consoleproxy.util.ImageHelper; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.util.TileInfo; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + /** * A BuffereImageCanvas component represents frame buffer image on * the screen. It also notifies its subscribers when screen is repainted. */ public class BufferedImageCanvas extends Canvas implements FrameBufferCanvas { private static final long serialVersionUID = 1L; - protected Logger logger = Logger.getLogger(BufferedImageCanvas.class); + protected Logger logger = LogManager.getLogger(BufferedImageCanvas.class); // Offline screen buffer private BufferedImage offlineImage; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java index c5764a994c5..e4bb93711b9 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java @@ -31,7 +31,6 @@ import java.security.spec.KeySpec; import java.util.Arrays; import java.util.List; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.util.RawHTTP; import com.cloud.consoleproxy.vnc.network.NioSocket; import com.cloud.consoleproxy.vnc.network.NioSocketHandler; @@ -42,7 +41,11 @@ import com.cloud.consoleproxy.vnc.security.VncTLSSecurity; import com.cloud.consoleproxy.websocket.WebSocketReverseProxy; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; + import org.apache.commons.lang3.BooleanUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import org.eclipse.jetty.websocket.api.Session; import javax.crypto.BadPaddingException; @@ -54,7 +57,7 @@ import javax.crypto.SecretKeyFactory; import javax.crypto.spec.DESKeySpec; public class NoVncClient { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private Socket socket; private DataInputStream is; @@ -79,6 +82,7 @@ public class NoVncClient { port = 80; } + logger.info("Connecting to VNC server {}:{} ...", host, port); RawHTTP tunnel = new RawHTTP("CONNECT", host, port, path, session, useSSL); socket = tunnel.connect(); setTunnelSocketStreams(); @@ -86,7 +90,7 @@ public class NoVncClient { public void connectTo(String host, int port) { // Connect to server - logger.info(String.format("Connecting to VNC server %s:%s ...", host, port)); + logger.info("Connecting to VNC server {}:{} ...", host, port); try { NioSocket nioSocket = new NioSocket(host, port); this.nioSocketConnection = new NioSocketHandlerImpl(nioSocket); @@ -175,8 +179,9 @@ public class NoVncClient { is.readFully(buf); String reason = new String(buf, RfbConstants.CHARSET); - logger.error("Authentication to VNC server is failed. Reason: " + reason); - throw new RuntimeException("Authentication to VNC server is failed. Reason: " + reason); + String msg = String.format("Authentication to VNC server has failed. Reason: %s", reason); + logger.error(msg); + throw new RuntimeException(msg); } case RfbConstants.NO_AUTH: { @@ -191,9 +196,9 @@ public class NoVncClient { } default: - logger.error("Unsupported VNC protocol authorization scheme, scheme code: " + authType + "."); - throw new RuntimeException( - "Unsupported VNC protocol authorization scheme, scheme code: " + authType + "."); + String msg = String.format("Unsupported VNC protocol authorization scheme, scheme code: %d.", authType); + logger.error(msg); + throw new RuntimeException(msg); } // Since we've taken care of the auth, we tell the client that there's no auth // going on diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClient.java index e5a9918d935..05bd01d390e 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClient.java @@ -33,13 +33,15 @@ import javax.crypto.SecretKeyFactory; import javax.crypto.spec.DESKeySpec; import com.cloud.consoleproxy.ConsoleProxyClientListener; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.util.RawHTTP; import com.cloud.consoleproxy.vnc.packet.client.KeyboardEventPacket; import com.cloud.consoleproxy.vnc.packet.client.MouseEventPacket; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class VncClient { - protected static Logger LOGGER = Logger.getLogger(VncClient.class); + protected static Logger LOGGER = LogManager.getLogger(VncClient.class); private Socket socket; private DataInputStream is; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClientPacketSender.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClientPacketSender.java index 12daca619ce..3d055b4dbb4 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClientPacketSender.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncClientPacketSender.java @@ -27,7 +27,6 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.vnc.packet.client.ClientPacket; import com.cloud.consoleproxy.vnc.packet.client.FramebufferUpdateRequestPacket; import com.cloud.consoleproxy.vnc.packet.client.KeyboardEventPacket; @@ -35,8 +34,11 @@ import com.cloud.consoleproxy.vnc.packet.client.MouseEventPacket; import com.cloud.consoleproxy.vnc.packet.client.SetEncodingsPacket; import com.cloud.consoleproxy.vnc.packet.client.SetPixelFormatPacket; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class VncClientPacketSender implements Runnable, PaintNotificationListener, KeyListener, MouseListener, MouseMotionListener, FrameBufferUpdateListener { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); // Queue for outgoing packets private final BlockingQueue queue = new ArrayBlockingQueue(30); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncServerPacketReceiver.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncServerPacketReceiver.java index effcb7b4599..bbd39754ac3 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncServerPacketReceiver.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/VncServerPacketReceiver.java @@ -22,12 +22,14 @@ import java.io.DataInputStream; import java.io.IOException; import com.cloud.consoleproxy.ConsoleProxyClientListener; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.vnc.packet.server.FramebufferUpdatePacket; import com.cloud.consoleproxy.vnc.packet.server.ServerCutText; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class VncServerPacketReceiver implements Runnable { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private final VncScreenDescription screen; private BufferedImageCanvas canvas; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/AbstractRect.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/AbstractRect.java index 2059278905b..a56ee63d306 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/AbstractRect.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/AbstractRect.java @@ -16,11 +16,12 @@ // under the License. package com.cloud.consoleproxy.vnc.packet.server; -import com.cloud.consoleproxy.util.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public abstract class AbstractRect implements Rect { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); protected final int x; protected final int y; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/ServerCutText.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/ServerCutText.java index 79ed98cccd0..76e47ab0676 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/ServerCutText.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/packet/server/ServerCutText.java @@ -19,11 +19,13 @@ package com.cloud.consoleproxy.vnc.packet.server; import java.io.DataInputStream; import java.io.IOException; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.vnc.RfbConstants; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class ServerCutText { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private String content; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncAuthSecurity.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncAuthSecurity.java index 29c29f8ff58..42f109b5cf3 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncAuthSecurity.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncAuthSecurity.java @@ -16,7 +16,6 @@ // under the License. package com.cloud.consoleproxy.vnc.security; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.vnc.NoVncClient; import com.cloud.consoleproxy.vnc.network.NioSocketHandler; import com.cloud.utils.exception.CloudRuntimeException; @@ -24,12 +23,15 @@ import com.cloud.utils.exception.CloudRuntimeException; import java.io.IOException; import java.nio.ByteBuffer; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class VncAuthSecurity implements VncSecurity { private final String vmPass; private static final int VNC_AUTH_CHALLENGE_SIZE = 16; - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); public VncAuthSecurity(String vmPass) { this.vmPass = vmPass; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncTLSSecurity.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncTLSSecurity.java index 00497a37828..0bc1355c4fb 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncTLSSecurity.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncTLSSecurity.java @@ -16,7 +16,6 @@ // under the License. package com.cloud.consoleproxy.vnc.security; -import com.cloud.consoleproxy.util.Logger; import com.cloud.consoleproxy.vnc.RfbConstants; import com.cloud.consoleproxy.vnc.network.NioSocketHandler; import com.cloud.consoleproxy.vnc.network.NioSocketSSLEngineManager; @@ -29,9 +28,12 @@ import java.io.IOException; import java.security.GeneralSecurityException; import java.util.ArrayList; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class VncTLSSecurity implements VncSecurity { - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private SSLContext ctx; private SSLEngine engine; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/websocket/WebSocketReverseProxy.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/websocket/WebSocketReverseProxy.java index 582fb625f2c..fecbaae38e8 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/websocket/WebSocketReverseProxy.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/websocket/WebSocketReverseProxy.java @@ -16,7 +16,6 @@ // under the License. package com.cloud.consoleproxy.websocket; -import com.cloud.consoleproxy.util.Logger; import org.eclipse.jetty.websocket.api.Session; import org.java_websocket.client.WebSocketClient; import org.java_websocket.drafts.Draft_6455; @@ -36,6 +35,9 @@ import java.nio.ByteBuffer; import java.security.cert.X509Certificate; import java.util.Collections; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + /** * Acts as a websocket reverse proxy between the remoteSession and the connected endpoint * - Connects to a websocket endpoint and sends the received data to the remoteSession endpoint @@ -51,7 +53,7 @@ public class WebSocketReverseProxy extends WebSocketClient { private static final DefaultExtension defaultExtension = new DefaultExtension(); private static final Draft_6455 draft = new Draft_6455(Collections.singletonList(defaultExtension), Collections.singletonList(protocol)); - protected Logger logger = Logger.getLogger(getClass()); + protected Logger logger = LogManager.getLogger(getClass()); private Session remoteSession; private void acceptAllCerts() { From b1f683db6b8575e76bd8f674c3169591579f61ee Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 10 Sep 2024 10:37:30 +0200 Subject: [PATCH 07/10] Allow more generic searches of ACLs (#9566) --- .../network/vpc/NetworkACLServiceImpl.java | 13 ++++++++++--- ui/src/views/network/AclListRulesTab.vue | 18 ++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java index 82e8462a559..54338173282 100644 --- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java @@ -697,6 +697,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ final String trafficType = cmd.getTrafficType(); final String protocol = cmd.getProtocol(); final String action = cmd.getAction(); + final String keyword = cmd.getKeyword(); final Map tags = cmd.getTags(); final Account caller = CallContext.current().getCallingAccount(); @@ -708,6 +709,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ sb.and("trafficType", sb.entity().getTrafficType(), Op.EQ); sb.and("protocol", sb.entity().getProtocol(), Op.EQ); sb.and("action", sb.entity().getAction(), Op.EQ); + sb.and("reason", sb.entity().getReason(), Op.EQ); if (tags != null && !tags.isEmpty()) { final SearchBuilder tagSearch = _resourceTagDao.createSearchBuilder(); @@ -730,6 +732,12 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ final SearchCriteria sc = sb.create(); + if (StringUtils.isNotBlank(keyword)) { + final SearchCriteria ssc = _networkACLItemDao.createSearchCriteria(); + ssc.addOr("protocol", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + ssc.addOr("reason", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + sc.addAnd("acl_id", SearchCriteria.Op.SC, ssc); + } if (id != null) { sc.setParameters("id", id); } @@ -747,7 +755,6 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ if (trafficType != null) { sc.setParameters("trafficType", trafficType); } - if (aclId != null) { // Get VPC and check access final NetworkACL acl = _networkACLDao.findById(aclId); @@ -764,7 +771,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ // aclId is not specified // List permitted VPCs and filter aclItems - final List permittedAccounts = new ArrayList(); + final List permittedAccounts = new ArrayList<>(); Long domainId = cmd.getDomainId(); boolean isRecursive = cmd.isRecursive(); final String accountName = cmd.getAccountName(); @@ -780,7 +787,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ final SearchCriteria scVpc = sbVpc.create(); _accountMgr.buildACLSearchCriteria(scVpc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); final List vpcs = _vpcDao.search(scVpc, null); - final List vpcIds = new ArrayList(); + final List vpcIds = new ArrayList<>(); for (final VpcVO vpc : vpcs) { vpcIds.add(vpc.getId()); } diff --git a/ui/src/views/network/AclListRulesTab.vue b/ui/src/views/network/AclListRulesTab.vue index fb0495b6c75..5303a97aefb 100644 --- a/ui/src/views/network/AclListRulesTab.vue +++ b/ui/src/views/network/AclListRulesTab.vue @@ -17,7 +17,8 @@ {{ $t('label.acl.export') }} +
+ +
@@ -324,6 +333,7 @@ export default { }, data () { return { + searchQuery: '', // Bind this to the search input acls: [], fetchLoading: false, protocolNumbers: [], @@ -433,7 +443,11 @@ export default { }, fetchData () { this.fetchLoading = true - api('listNetworkACLs', { aclid: this.resource.id }).then(json => { + const params = { + aclid: this.resource.id, + keyword: this.searchQuery + } + api('listNetworkACLs', params).then(json => { this.acls = json.listnetworkaclsresponse.networkacl || [] if (this.acls.length > 0) { this.acls.sort((a, b) => a.number - b.number) From 6ec3c486ddb4d5daeba59e68e2634a446564f6dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Tue, 10 Sep 2024 08:12:41 -0300 Subject: [PATCH 08/10] Enhance the `listAffinityGroups` API by adding the dedicated resources related to an affinity group (#9188) * add dedicated resource response * populate dedicatedresources field * change affinity group name and description when it contains dedicated resources * display dedicatedresources on UI * add end of line to DedicatedResourceResponse class * remove unnecessary fully qualified names --- .../java/com/cloud/dc/DedicatedResources.java | 4 ++ .../affinity/AffinityGroupResponse.java | 13 ++++ .../dedicated/DedicatedResourceResponse.java | 44 +++++++++++++ .../DedicatedResourceManagerImpl.java | 36 +++++------ .../query/dao/AffinityGroupJoinDaoImpl.java | 61 ++++++++++++++++++- ui/public/locales/en.json | 1 + ui/public/locales/pt_BR.json | 1 + ui/src/components/view/DetailsTab.vue | 20 ++++++ ui/src/config/section/compute.js | 2 +- 9 files changed, 163 insertions(+), 19 deletions(-) create mode 100644 api/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceResponse.java diff --git a/api/src/main/java/com/cloud/dc/DedicatedResources.java b/api/src/main/java/com/cloud/dc/DedicatedResources.java index 63188ca0b0e..23e6cc88a1e 100644 --- a/api/src/main/java/com/cloud/dc/DedicatedResources.java +++ b/api/src/main/java/com/cloud/dc/DedicatedResources.java @@ -21,6 +21,10 @@ import org.apache.cloudstack.api.Identity; import org.apache.cloudstack.api.InternalIdentity; public interface DedicatedResources extends InfrastructureEntity, InternalIdentity, Identity { + enum Type { + Zone, Pod, Cluster, Host + } + @Override long getId(); diff --git a/api/src/main/java/org/apache/cloudstack/affinity/AffinityGroupResponse.java b/api/src/main/java/org/apache/cloudstack/affinity/AffinityGroupResponse.java index 6cda8d07bd8..69f391a5656 100644 --- a/api/src/main/java/org/apache/cloudstack/affinity/AffinityGroupResponse.java +++ b/api/src/main/java/org/apache/cloudstack/affinity/AffinityGroupResponse.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; import org.apache.cloudstack.api.EntityReference; import org.apache.cloudstack.api.response.ControlledViewEntityResponse; +import org.apache.cloudstack.dedicated.DedicatedResourceResponse; import com.cloud.serializer.Param; @@ -76,6 +77,10 @@ public class AffinityGroupResponse extends BaseResponse implements ControlledVie @Param(description = "virtual machine IDs associated with this affinity group") private List vmIdList; + @SerializedName("dedicatedresources") + @Param(description = "dedicated resources associated with this affinity group") + private List dedicatedResources; + public AffinityGroupResponse() { } @@ -171,4 +176,12 @@ public class AffinityGroupResponse extends BaseResponse implements ControlledVie this.vmIdList.add(vmId); } + public void addDedicatedResource(DedicatedResourceResponse dedicatedResourceResponse) { + if (this.dedicatedResources == null) { + this.dedicatedResources = new ArrayList<>(); + } + + this.dedicatedResources.add(dedicatedResourceResponse); + } + } diff --git a/api/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceResponse.java b/api/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceResponse.java new file mode 100644 index 00000000000..fc116cbb91e --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceResponse.java @@ -0,0 +1,44 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.dedicated; + +import com.cloud.dc.DedicatedResources; +import com.cloud.serializer.Param; + +import com.google.gson.annotations.SerializedName; + +import org.apache.cloudstack.api.BaseResponse; + +public class DedicatedResourceResponse extends BaseResponse { + @SerializedName("resourceid") + @Param(description = "the ID of the resource") + private String resourceId; + + @SerializedName("resourcename") + @Param(description = "the name of the resource") + private String resourceName; + + @SerializedName("resourcetype") + @Param(description = "the type of the resource") + private DedicatedResources.Type resourceType; + + public DedicatedResourceResponse(String resourceId, String resourceName, DedicatedResources.Type resourceType) { + this.resourceId = resourceId; + this.resourceName = resourceName; + this.resourceType = resourceType; + } +} diff --git a/plugins/dedicated-resources/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceManagerImpl.java b/plugins/dedicated-resources/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceManagerImpl.java index 9060eccb64a..4f1db396b7c 100644 --- a/plugins/dedicated-resources/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceManagerImpl.java +++ b/plugins/dedicated-resources/src/main/java/org/apache/cloudstack/dedicated/DedicatedResourceManagerImpl.java @@ -23,6 +23,7 @@ import java.util.Map; import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.commons.lang3.StringUtils; import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupService; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; @@ -236,7 +237,7 @@ public class DedicatedResourceManagerImpl implements DedicatedService { @Override public List doInTransaction(TransactionStatus status) { // find or create the affinity group by name under this account/domain - AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal); + AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal, DedicatedResources.Type.Zone); if (group == null) { logger.error("Unable to dedicate zone due to, failed to create dedication affinity group"); throw new CloudRuntimeException("Failed to dedicate zone. Please contact Cloud Support."); @@ -372,10 +373,10 @@ public class DedicatedResourceManagerImpl implements DedicatedService { @Override public List doInTransaction(TransactionStatus status) { // find or create the affinity group by name under this account/domain - AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal); + AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal, DedicatedResources.Type.Pod); if (group == null) { - logger.error("Unable to dedicate zone due to, failed to create dedication affinity group"); - throw new CloudRuntimeException("Failed to dedicate zone. Please contact Cloud Support."); + logger.error("Unable to dedicate pod due to, failed to create dedication affinity group"); + throw new CloudRuntimeException("Failed to dedicate pod. Please contact Cloud Support."); } DedicatedResourceVO dedicatedResource = new DedicatedResourceVO(null, podId, null, null, null, null, group.getId()); try { @@ -485,10 +486,10 @@ public class DedicatedResourceManagerImpl implements DedicatedService { @Override public List doInTransaction(TransactionStatus status) { // find or create the affinity group by name under this account/domain - AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal); + AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal, DedicatedResources.Type.Cluster); if (group == null) { - logger.error("Unable to dedicate zone due to, failed to create dedication affinity group"); - throw new CloudRuntimeException("Failed to dedicate zone. Please contact Cloud Support."); + logger.error("Unable to dedicate cluster due to, failed to create dedication affinity group"); + throw new CloudRuntimeException("Failed to dedicate cluster. Please contact Cloud Support."); } DedicatedResourceVO dedicatedResource = new DedicatedResourceVO(null, null, clusterId, null, null, null, group.getId()); try { @@ -582,10 +583,10 @@ public class DedicatedResourceManagerImpl implements DedicatedService { @Override public List doInTransaction(TransactionStatus status) { // find or create the affinity group by name under this account/domain - AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal); + AffinityGroup group = findOrCreateDedicatedAffinityGroup(domainId, accountIdFinal, DedicatedResources.Type.Host); if (group == null) { - logger.error("Unable to dedicate zone due to, failed to create dedication affinity group"); - throw new CloudRuntimeException("Failed to dedicate zone. Please contact Cloud Support."); + logger.error("Unable to dedicate host due to, failed to create dedication affinity group"); + throw new CloudRuntimeException("Failed to dedicate host. Please contact Cloud Support."); } DedicatedResourceVO dedicatedResource = new DedicatedResourceVO(null, null, null, hostId, null, null, group.getId()); try { @@ -607,7 +608,7 @@ public class DedicatedResourceManagerImpl implements DedicatedService { } - private AffinityGroup findOrCreateDedicatedAffinityGroup(Long domainId, Long accountId) { + private AffinityGroup findOrCreateDedicatedAffinityGroup(Long domainId, Long accountId, DedicatedResources.Type dedicatedResource) { if (domainId == null) { return null; } @@ -624,24 +625,25 @@ public class DedicatedResourceManagerImpl implements DedicatedService { if (group != null) { return group; } - // default to a groupname with account/domain information - affinityGroupName = "DedicatedGrp-" + accountName; + // defaults to a groupName with resourceType and account/domain information + affinityGroupName = String.format("Dedicated%sGrp-%s", dedicatedResource, accountName); } else { // domain level group group = _affinityGroupDao.findDomainLevelGroupByType(domainId, "ExplicitDedication"); if (group != null) { return group; } - // default to a groupname with account/domain information + + // defaults to a groupName with resourceType and account/domain information String domainName = _domainDao.findById(domainId).getName(); - affinityGroupName = "DedicatedGrp-domain-" + domainName; + affinityGroupName = String.format("Dedicated%sGrp-domain-%s", dedicatedResource, domainName); } - group = _affinityGroupService.createAffinityGroup(accountName, null, domainId, affinityGroupName, "ExplicitDedication", "dedicated resources group"); + String description = String.format("Dedicated %s group", StringUtils.lowerCase(dedicatedResource.toString())); + group = _affinityGroupService.createAffinityGroup(accountName, null, domainId, affinityGroupName, "ExplicitDedication", description); return group; - } private List getVmsOnHost(long hostId) { diff --git a/server/src/main/java/com/cloud/api/query/dao/AffinityGroupJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AffinityGroupJoinDaoImpl.java index 2a876ea8226..a5fd2bf11f1 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AffinityGroupJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AffinityGroupJoinDaoImpl.java @@ -21,21 +21,46 @@ import java.util.List; import javax.inject.Inject; - import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupResponse; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.dedicated.DedicatedResourceResponse; import com.cloud.api.ApiResponseHelper; import com.cloud.api.query.vo.AffinityGroupJoinVO; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; +import com.cloud.dc.DataCenter; +import com.cloud.dc.DedicatedResourceVO; +import com.cloud.dc.DedicatedResources; +import com.cloud.dc.HostPodVO; +import com.cloud.dc.dao.ClusterDao; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.DedicatedResourceDao; +import com.cloud.dc.dao.HostPodDao; +import com.cloud.host.Host; +import com.cloud.host.dao.HostDao; +import com.cloud.org.Cluster; +import com.cloud.user.AccountManager; public class AffinityGroupJoinDaoImpl extends GenericDaoBase implements AffinityGroupJoinDao { @Inject private ConfigurationDao _configDao; + @Inject + private DedicatedResourceDao dedicatedResourceDao; + @Inject + private DataCenterDao dataCenterDao; + @Inject + private HostPodDao podDao; + @Inject + private ClusterDao clusterDao; + @Inject + private HostDao hostDao; + @Inject + private AccountManager accountManager; private final SearchBuilder agSearch; @@ -64,6 +89,14 @@ public class AffinityGroupJoinDaoImpl extends GenericDaoBase dedicatedResources = dedicatedResourceDao.listByAffinityGroupId(vag.getId()); + this.populateDedicatedResourcesField(dedicatedResources, agResponse); + } + // update vm information long instanceId = vag.getVmId(); if (instanceId > 0) { @@ -76,6 +109,32 @@ public class AffinityGroupJoinDaoImpl extends GenericDaoBase dedicatedResources, AffinityGroupResponse agResponse) { + if (dedicatedResources.isEmpty()) { + return; + } + + for (DedicatedResourceVO resource : dedicatedResources) { + DedicatedResourceResponse dedicatedResourceResponse = null; + + if (resource.getDataCenterId() != null) { + DataCenter dataCenter = dataCenterDao.findById(resource.getDataCenterId()); + dedicatedResourceResponse = new DedicatedResourceResponse(dataCenter.getUuid(), dataCenter.getName(), DedicatedResources.Type.Zone); + } else if (resource.getPodId() != null) { + HostPodVO pod = podDao.findById(resource.getPodId()); + dedicatedResourceResponse = new DedicatedResourceResponse(pod.getUuid(), pod.getName(), DedicatedResources.Type.Pod); + } else if (resource.getClusterId() != null) { + Cluster cluster = clusterDao.findById(resource.getClusterId()); + dedicatedResourceResponse = new DedicatedResourceResponse(cluster.getUuid(), cluster.getName(), DedicatedResources.Type.Cluster); + } else if (resource.getHostId() != null) { + Host host = hostDao.findById(resource.getHostId()); + dedicatedResourceResponse = new DedicatedResourceResponse(host.getUuid(), host.getName(), DedicatedResources.Type.Host); + } + + agResponse.addDedicatedResource(dedicatedResourceResponse); + } + } + @Override public AffinityGroupResponse setAffinityGroupResponse(AffinityGroupResponse vagData, AffinityGroupJoinVO vag) { // update vm information diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 6ca710d4095..7336c038c89 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -673,6 +673,7 @@ "label.dedicate.zone": "Dedicate zone", "label.dedicated": "Dedicated", "label.dedicated.vlan.vni.ranges": "Dedicated VLAN/VNI ranges", +"label.dedicatedresources": "Dedicated resources", "label.default": "Default", "label.default.use": "Default use", "label.default.view": "Default view", diff --git a/ui/public/locales/pt_BR.json b/ui/public/locales/pt_BR.json index 79333c100d3..f02aee747eb 100644 --- a/ui/public/locales/pt_BR.json +++ b/ui/public/locales/pt_BR.json @@ -472,6 +472,7 @@ "label.dedicate.zone": "Zona dedicada", "label.dedicated": "Dedicado", "label.dedicated.vlan.vni.ranges": "Intervalo(s) de VLAN/VNI dedicados", +"label.dedicatedresources": "Recursos dedicados", "label.default": "Padr\u00e3o", "label.default.use": "Uso padr\u00e3o", "label.default.view": "Visualiza\u00e7\u00e3o padr\u00e3o", diff --git a/ui/src/components/view/DetailsTab.vue b/ui/src/components/view/DetailsTab.vue index 3f6f37aa08b..f5f180c8f19 100644 --- a/ui/src/components/view/DetailsTab.vue +++ b/ui/src/components/view/DetailsTab.vue @@ -115,6 +115,15 @@
{{ JSON.stringify(JSON.parse(dataResource[item]), null, 4) || dataResource[item] }}
+
+
+
+ + {{ resource.resourcename }} + +
+
+
{{ dataResource[item] }}
@@ -150,6 +159,7 @@ import DedicateData from './DedicateData' import HostInfo from '@/views/infra/HostInfo' import VmwareData from './VmwareData' +import { genericCompare } from '@/utils/sort' export default { name: 'DetailsTab', @@ -386,6 +396,16 @@ export default { }, getDetailTitle (detail) { return `label.${String(this.detailsTitles[detail]).toLowerCase()}` + }, + getResourceLink (type, id) { + return `/${type.toLowerCase()}/${id}` + }, + sortDedicatedResourcesByName (resources) { + resources.sort((resource, otherResource) => { + return genericCompare(resource.resourcename, otherResource.resourcename) + }) + + return resources } } } diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index b992229962b..c6d16d6ea78 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -988,7 +988,7 @@ export default { } return fields }, - details: ['name', 'id', 'description', 'type', 'account', 'domain'], + details: ['name', 'id', 'description', 'type', 'account', 'domain', 'dedicatedresources'], related: [{ name: 'vm', title: 'label.instances', From 638c1526d0a71fc422ffa9ed694afde3d51e1bc5 Mon Sep 17 00:00:00 2001 From: Thomas O'Dowd Date: Tue, 10 Sep 2024 21:52:59 +0900 Subject: [PATCH 09/10] Fix the Cloudian Integration SSO Redirect link (#9656) --- .../cloudian/client/CloudianUtils.java | 8 +- .../cloudian/CloudianUtilsTest.java | 87 +++++++++++++++++++ 2 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 plugins/integrations/cloudian/src/test/java/org/apache/cloudstack/cloudian/CloudianUtilsTest.java diff --git a/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/client/CloudianUtils.java b/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/client/CloudianUtils.java index 882d615ca0b..6d70e785d0a 100644 --- a/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/client/CloudianUtils.java +++ b/plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/client/CloudianUtils.java @@ -81,12 +81,8 @@ public class CloudianUtils { return null; } - stringBuilder.append("&redirect="); - if (group.equals("0")) { - stringBuilder.append("admin.htm"); - } else { - stringBuilder.append("explorer.htm"); - } + // Redirects to dashboard for admin users or the bucket browser for regular users + stringBuilder.append("&redirect=/"); return cmcUrlPath + "ssosecurelogin.htm?" + stringBuilder.toString(); } diff --git a/plugins/integrations/cloudian/src/test/java/org/apache/cloudstack/cloudian/CloudianUtilsTest.java b/plugins/integrations/cloudian/src/test/java/org/apache/cloudstack/cloudian/CloudianUtilsTest.java new file mode 100644 index 00000000000..4bc9ce1fe28 --- /dev/null +++ b/plugins/integrations/cloudian/src/test/java/org/apache/cloudstack/cloudian/CloudianUtilsTest.java @@ -0,0 +1,87 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.cloudian; + +import java.io.UnsupportedEncodingException; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLDecoder; +import java.util.HashMap; +import org.apache.cloudstack.cloudian.client.CloudianUtils; +import org.junit.Assert; +import org.junit.Test; + +public class CloudianUtilsTest { + + @Test + public void testGenerateSSOUrl() { + final String cmcUrlPath = "https://cmc.cloudian.com:8443/Cloudian/"; + final String user = "abc-def-ghi"; + final String group = "uvw-xyz"; + final String ssoKey = "randomkey"; + + // test expectations + final String expPath = "/Cloudian/ssosecurelogin.htm"; + HashMap expected = new HashMap(); + expected.put("user", user); + expected.put("group", group); + expected.put("timestamp", null); // null value will not be checked by this test + expected.put("signature", null); // null value will not be checked by this test + expected.put("redirect", "/"); + + // Generated URL will be something like this + // https://cmc.cloudian.com:8443/Cloudian/ssosecurelogin.htm?user=abc-def-ghi&group=uvw-xyz×tamp=1725937474949&signature=Wu1hjafeyE82mGwd1MIwrp5hPt4%3D&redirect=/ + String output = CloudianUtils.generateSSOUrl(cmcUrlPath, user, group, ssoKey); + Assert.assertNotNull(output); + + // Check main parts of the output URL + URL url = null; + try { + url = new URL(output); + } catch (MalformedURLException e) { + Assert.fail("failed to parse URL: " + output); + } + String path = url.getPath(); + Assert.assertEquals(expPath, path); + + // No easy way to check Query parameters in Java still? + // Just do a rudementary check as we are in charge of the URL + String query = url.getQuery(); + String[] nameValues = query.split("&"); + int matchedCount = 0; + for(String nameValue : nameValues) { + String[] nameValuePair = nameValue.split("=", 2); + Assert.assertEquals(nameValue, 2, nameValuePair.length); + String name = null; + String value = null; + try { + name = URLDecoder.decode(nameValuePair[0], "UTF-8"); + value = URLDecoder.decode(nameValuePair[1], "UTF-8"); + } catch (UnsupportedEncodingException e) { + Assert.fail("not expecting UTF-8 to fail"); + } + Assert.assertTrue(expected.containsKey(name)); + matchedCount++; + String expValue = expected.get(name); + if (expValue != null) { + Assert.assertEquals("Parameter " + name, expValue, value); + } + } + Assert.assertEquals("Should be 5 query parameters", 5, matchedCount); + } +} From 0655075f51c57cf031f9f9195f94cf2d2f3d8abc Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 10 Sep 2024 21:25:28 +0530 Subject: [PATCH 10/10] Feature: Forgot password (#9509) * Feature: Forgot password * Address comments * fixups * Make forgot password disabled by default * Apply suggestions from code review * Address comments --- .../cloudstack/api/ApiServerService.java | 6 + .../api/auth/APIAuthenticationType.java | 2 +- .../java/com/cloud/user/UserAccountVO.java | 4 + .../resourcedetail/UserDetailVO.java | 2 + .../management/MockAccountManager.java | 5 + pom.xml | 1 + server/pom.xml | 5 + .../main/java/com/cloud/api/ApiServer.java | 71 +++- .../auth/APIAuthenticationManagerImpl.java | 6 + ...aultForgotPasswordAPIAuthenticatorCmd.java | 165 +++++++++ ...faultResetPasswordAPIAuthenticatorCmd.java | 193 +++++++++++ .../java/com/cloud/user/AccountManager.java | 4 +- .../com/cloud/user/AccountManagerImpl.java | 9 +- .../user/UserPasswordResetManager.java | 71 ++++ .../user/UserPasswordResetManagerImpl.java | 312 +++++++++++++++++ .../spring-server-core-managers-context.xml | 1 + .../java/com/cloud/api/ApiServerTest.java | 92 ++++- .../cloud/user/AccountManagerImplTest.java | 20 +- .../cloud/user/MockAccountManagerImpl.java | 4 + .../UserPasswordResetManagerImplTest.java | 150 +++++++++ tools/apidoc/gen_toc.py | 4 +- ui/public/locales/en.json | 5 + ui/src/config/router.js | 10 + ui/src/permission.js | 2 +- ui/src/utils/request.js | 2 +- ui/src/views/auth/ForgotPassword.vue | 260 ++++++++++++++ ui/src/views/auth/Login.vue | 23 +- ui/src/views/auth/ResetPassword.vue | 318 ++++++++++++++++++ .../utils/mailing/SMTPMailSender.java | 20 +- 29 files changed, 1726 insertions(+), 41 deletions(-) create mode 100644 server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java create mode 100644 server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java create mode 100644 server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java create mode 100644 server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java create mode 100644 server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java create mode 100644 ui/src/views/auth/ForgotPassword.vue create mode 100644 ui/src/views/auth/ResetPassword.vue diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java index 54fda7e36b8..cbbcdc3bda4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java @@ -21,7 +21,9 @@ import java.util.Map; import javax.servlet.http.HttpSession; +import com.cloud.domain.Domain; import com.cloud.exception.CloudAuthenticationException; +import com.cloud.user.UserAccount; public interface ApiServerService { public boolean verifyRequest(Map requestParameters, Long userId, InetAddress remoteAddress) throws ServerApiException; @@ -42,4 +44,8 @@ public interface ApiServerService { public String handleRequest(Map params, String responseType, StringBuilder auditTrailSb) throws ServerApiException; public Class getCmdClass(String cmdName); + + boolean forgotPassword(UserAccount userAccount, Domain domain); + + boolean resetPassword(UserAccount userAccount, String token, String password); } diff --git a/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java b/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java index 5ba9d182daa..1f78708f7e5 100644 --- a/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java +++ b/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java @@ -17,5 +17,5 @@ package org.apache.cloudstack.api.auth; public enum APIAuthenticationType { - LOGIN_API, LOGOUT_API, READONLY_API, LOGIN_2FA_API + LOGIN_API, LOGOUT_API, READONLY_API, LOGIN_2FA_API, PASSWORD_RESET } diff --git a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java index c18ca53f7ab..1da7d52a366 100644 --- a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java +++ b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java @@ -17,6 +17,7 @@ package com.cloud.user; import java.util.Date; +import java.util.HashMap; import java.util.Map; import javax.persistence.Column; @@ -361,6 +362,9 @@ public class UserAccountVO implements UserAccount, InternalIdentity { @Override public Map getDetails() { + if (details == null) { + details = new HashMap<>(); + } return details; } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java index 1b430e806e2..d0cfcc3d439 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java @@ -46,6 +46,8 @@ public class UserDetailVO implements ResourceDetail { private boolean display = true; public static final String Setup2FADetail = "2FASetupStatus"; + public static final String PasswordResetToken = "PasswordResetToken"; + public static final String PasswordResetTokenExpiryDate = "PasswordResetTokenExpiryDate"; public UserDetailVO() { } diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index 5d2efa0dc9a..6bb9752d764 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -515,6 +515,11 @@ public class MockAccountManager extends ManagerBase implements AccountManager { return null; } + public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, + String currentPassword, + boolean skipCurrentPassValidation) { + } + @Override public void checkApiAccess(Account account, String command) throws PermissionDeniedException { diff --git a/pom.xml b/pom.xml index ff6cac2ff6a..29fc939f553 100644 --- a/pom.xml +++ b/pom.xml @@ -169,6 +169,7 @@ 2.7.0 0.5.3 1.5.0-b01 + 0.9.14 8.0.33 2.0.4 10.1 diff --git a/server/pom.xml b/server/pom.xml index ec157b00e30..6b027b2c7c7 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -101,6 +101,11 @@ commons-math3 ${cs.commons-math3.version} + + com.github.spullara.mustache.java + compiler + ${cs.mustache.version} + org.apache.cloudstack cloud-utils diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 0d4382097c2..739ad765afa 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -55,6 +55,13 @@ import javax.naming.ConfigurationException; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.user.AccountManagerImpl; +import com.cloud.user.DomainManager; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.user.UserVO; import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -103,7 +110,9 @@ import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.MessageDispatcher; import org.apache.cloudstack.framework.messagebus.MessageHandler; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.user.UserPasswordResetManager; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang3.EnumUtils; import org.apache.http.ConnectionClosedException; import org.apache.http.HttpException; import org.apache.http.HttpRequest; @@ -157,13 +166,6 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.UnavailableCommandException; import com.cloud.projects.dao.ProjectDao; import com.cloud.storage.VolumeApiService; -import com.cloud.user.Account; -import com.cloud.user.AccountManager; -import com.cloud.user.AccountManagerImpl; -import com.cloud.user.DomainManager; -import com.cloud.user.User; -import com.cloud.user.UserAccount; -import com.cloud.user.UserVO; import com.cloud.utils.ConstantTimeComparator; import com.cloud.utils.DateUtil; import com.cloud.utils.HttpUtils; @@ -182,6 +184,8 @@ import com.cloud.utils.exception.ExceptionProxyObject; import com.cloud.utils.net.NetUtils; import com.google.gson.reflect.TypeToken; +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + @Component public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable { @@ -214,6 +218,8 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer private ProjectDao projectDao; @Inject private UUIDManager uuidMgr; + @Inject + private UserPasswordResetManager userPasswordResetManager; private List pluggableServices; @@ -1223,6 +1229,57 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer return true; } + @Override + public boolean forgotPassword(UserAccount userAccount, Domain domain) { + if (!UserPasswordResetEnabled.value()) { + String errorMessage = String.format("%s is false. Password reset for the user is not allowed.", + UserPasswordResetEnabled.key()); + logger.error(errorMessage); + throw new CloudRuntimeException(errorMessage); + } + if (StringUtils.isBlank(userAccount.getEmail())) { + logger.error(String.format( + "Email is not set. username: %s account id: %d domain id: %d", + userAccount.getUsername(), userAccount.getAccountId(), userAccount.getDomainId())); + throw new CloudRuntimeException("Email is not set for the user."); + } + + if (!EnumUtils.getEnumIgnoreCase(Account.State.class, userAccount.getState()).equals(Account.State.ENABLED)) { + logger.error(String.format( + "User is not enabled. username: %s account id: %d domain id: %s", + userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid())); + throw new CloudRuntimeException("User is not enabled."); + } + + if (!EnumUtils.getEnumIgnoreCase(Account.State.class, userAccount.getAccountState()).equals(Account.State.ENABLED)) { + logger.error(String.format( + "Account is not enabled. username: %s account id: %d domain id: %s", + userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid())); + throw new CloudRuntimeException("Account is not enabled."); + } + + if (!domain.getState().equals(Domain.State.Active)) { + logger.error(String.format( + "Domain is not active. username: %s account id: %d domain id: %s", + userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid())); + throw new CloudRuntimeException("Domain is not active."); + } + + userPasswordResetManager.setResetTokenAndSend(userAccount); + return true; + } + + @Override + public boolean resetPassword(UserAccount userAccount, String token, String password) { + if (!UserPasswordResetEnabled.value()) { + String errorMessage = String.format("%s is false. Password reset for the user is not allowed.", + UserPasswordResetEnabled.key()); + logger.error(errorMessage); + throw new CloudRuntimeException(errorMessage); + } + return userPasswordResetManager.validateAndResetPassword(userAccount, token, password); + } + private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException { if (user == null) { throw new PermissionDeniedException("User is null for role based API access check for command" + commandName); diff --git a/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java b/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java index 907ef088ee8..3c8282d0280 100644 --- a/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java +++ b/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java @@ -31,6 +31,8 @@ import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator; import com.cloud.utils.component.ComponentContext; import com.cloud.utils.component.ManagerBase; +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + @SuppressWarnings("unchecked") public class APIAuthenticationManagerImpl extends ManagerBase implements APIAuthenticationManager { @@ -75,6 +77,10 @@ public class APIAuthenticationManagerImpl extends ManagerBase implements APIAuth List> cmdList = new ArrayList>(); cmdList.add(DefaultLoginAPIAuthenticatorCmd.class); cmdList.add(DefaultLogoutAPIAuthenticatorCmd.class); + if (UserPasswordResetEnabled.value()) { + cmdList.add(DefaultForgotPasswordAPIAuthenticatorCmd.class); + cmdList.add(DefaultResetPasswordAPIAuthenticatorCmd.class); + } cmdList.add(ListUserTwoFactorAuthenticatorProvidersCmd.class); cmdList.add(ValidateUserTwoFactorAuthenticationCodeCmd.class); diff --git a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java new file mode 100644 index 00000000000..1e90b43c5e8 --- /dev/null +++ b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java @@ -0,0 +1,165 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.api.auth; + +import com.cloud.api.ApiServlet; +import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.domain.Domain; +import com.cloud.user.Account; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ApiServerService; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.auth.APIAuthenticationType; +import org.apache.cloudstack.api.auth.APIAuthenticator; +import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.jetbrains.annotations.Nullable; + +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import java.net.InetAddress; +import java.util.List; +import java.util.Map; + +@APICommand(name = "forgotPassword", + description = "Sends an email to the user with a token to reset the password using resetPassword command.", + since = "4.20.0.0", + requestHasSensitiveInfo = true, + responseObject = SuccessResponse.class) +public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { + + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + @Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, description = "Username", required = true) + private String username; + + @Parameter(name = ApiConstants.DOMAIN, type = CommandType.STRING, description = "Path of the domain that the user belongs to. Example: domain=/com/cloud/internal. If no domain is passed in, the ROOT (/) domain is assumed.") + private String domain; + + @Inject + ApiServerService _apiServer; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public String getUsername() { + return username; + } + + public String getDomainName() { + return domain; + } + + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public long getEntityOwnerId() { + return Account.Type.NORMAL.ordinal(); + } + + @Override + public void execute() throws ServerApiException { + // We should never reach here + throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "This is an authentication api, cannot be used directly"); + } + + @Override + public String authenticate(String command, Map params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException { + final String[] username = (String[])params.get(ApiConstants.USERNAME); + final String[] domainName = (String[])params.get(ApiConstants.DOMAIN); + + Long domainId = null; + String domain = null; + domain = getDomainName(auditTrailSb, domainName, domain); + + String serializedResponse = null; + if (username != null) { + try { + final Domain userDomain = _domainService.findDomainByPath(domain); + if (userDomain != null) { + domainId = userDomain.getId(); + } else { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find the domain from the path %s", domain)); + } + final UserAccount userAccount = _accountService.getActiveUserAccount(username[0], domainId); + if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Forgot Password is not allowed for this user"); + } + boolean success = _apiServer.forgotPassword(userAccount, userDomain); + logger.debug("Forgot password request for user " + username[0] + " in domain " + domain + " is successful: " + success); + } catch (final CloudRuntimeException ex) { + ApiServlet.invalidateHttpSession(session, "fall through to API key,"); + String msg = String.format("%s", ex.getMessage() != null ? + ex.getMessage() : + "forgot password request failed for user, check if username/domain are correct"); + auditTrailSb.append(" " + ApiErrorCode.ACCOUNT_ERROR + " " + msg); + serializedResponse = _apiServer.getSerializedApiError(ApiErrorCode.ACCOUNT_ERROR.getHttpCode(), msg, params, responseType); + if (logger.isTraceEnabled()) { + logger.trace(msg); + } + } + SuccessResponse successResponse = new SuccessResponse(); + successResponse.setSuccess(true); + successResponse.setResponseName(getCommandName()); + return ApiResponseSerializer.toSerializedString(successResponse, responseType); + } + // We should not reach here and if we do we throw an exception + throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, serializedResponse); + } + + @Nullable + private String getDomainName(StringBuilder auditTrailSb, String[] domainName, String domain) { + if (domainName != null) { + domain = domainName[0]; + auditTrailSb.append(" domain=" + domain); + if (domain != null) { + // ensure domain starts with '/' and ends with '/' + if (!domain.endsWith("/")) { + domain += '/'; + } + if (!domain.startsWith("/")) { + domain = "/" + domain; + } + } + } + return domain; + } + + @Override + public APIAuthenticationType getAPIType() { + return APIAuthenticationType.PASSWORD_RESET; + } + + @Override + public void setAuthenticators(List authenticators) { + } +} diff --git a/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java new file mode 100644 index 00000000000..077efdee087 --- /dev/null +++ b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java @@ -0,0 +1,193 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.api.auth; + +import com.cloud.api.ApiServlet; +import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.domain.Domain; +import com.cloud.exception.CloudAuthenticationException; +import com.cloud.user.Account; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.utils.UuidUtils; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ApiServerService; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.auth.APIAuthenticationType; +import org.apache.cloudstack.api.auth.APIAuthenticator; +import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.jetbrains.annotations.Nullable; + +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import java.net.InetAddress; +import java.util.List; +import java.util.Map; + +@APICommand(name = "resetPassword", + description = "Resets the password for the user using the token generated via forgotPassword command.", + since = "4.20.0.0", + requestHasSensitiveInfo = true, + responseObject = SuccessResponse.class) +public class DefaultResetPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { + + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + @Parameter(name = ApiConstants.USERNAME, + type = CommandType.STRING, + description = "Username", required = true) + private String username; + + @Parameter(name = ApiConstants.DOMAIN, + type = CommandType.STRING, + description = "Path of the domain that the user belongs to. Example: domain=/com/cloud/internal. If no domain is passed in, the ROOT (/) domain is assumed.") + private String domain; + + @Parameter(name = ApiConstants.TOKEN, + type = CommandType.STRING, + required = true, + description = "Token generated via forgotPassword command.") + private String token; + + @Parameter(name = ApiConstants.PASSWORD, + type = CommandType.STRING, + required = true, + description = "New password in clear text (Default hashed to SHA256SALT).") + private String password; + + @Inject + ApiServerService _apiServer; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public String getUsername() { + return username; + } + + public String getDomainName() { + return domain; + } + + public String getToken() { + return token; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public long getEntityOwnerId() { + return Account.Type.NORMAL.ordinal(); + } + + @Override + public void execute() throws ServerApiException { + // We should never reach here + throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "This is an authentication api, cannot be used directly"); + } + + @Override + public String authenticate(String command, Map params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException { + final String[] username = (String[])params.get(ApiConstants.USERNAME); + final String[] password = (String[])params.get(ApiConstants.PASSWORD); + final String[] domainName = (String[])params.get(ApiConstants.DOMAIN); + final String[] token = (String[])params.get(ApiConstants.TOKEN); + + Long domainId = null; + String domain = null; + domain = getDomainName(auditTrailSb, domainName, domain); + + String serializedResponse = null; + + if (!UuidUtils.isUuid(token[0])) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid token"); + } + + if (username != null) { + final String pwd = ((password == null) ? null : password[0]); + try { + final Domain userDomain = _domainService.findDomainByPath(domain); + if (userDomain != null) { + domainId = userDomain.getId(); + } else { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find the domain from the path %s", domain)); + } + final UserAccount userAccount = _accountService.getActiveUserAccount(username[0], domainId); + if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) { + throw new CloudAuthenticationException("Password reset is not allowed for CloudStack login"); + } + boolean success = _apiServer.resetPassword(userAccount, token[0], pwd); + SuccessResponse successResponse = new SuccessResponse(); + successResponse.setSuccess(success); + successResponse.setResponseName(getCommandName()); + return ApiResponseSerializer.toSerializedString(successResponse, responseType); + } catch (final CloudRuntimeException ex) { + ApiServlet.invalidateHttpSession(session, "fall through to API key,"); + String msg = String.format("%s", ex.getMessage() != null ? + ex.getMessage() : + "failed to reset password for user, check your inputs"); + auditTrailSb.append(" " + ApiErrorCode.ACCOUNT_ERROR + " " + msg); + serializedResponse = _apiServer.getSerializedApiError(ApiErrorCode.ACCOUNT_ERROR.getHttpCode(), msg, params, responseType); + if (logger.isTraceEnabled()) { + logger.trace(msg); + } + } + } + // We should not reach here and if we do we throw an exception + throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, serializedResponse); + } + + @Nullable + private String getDomainName(StringBuilder auditTrailSb, String[] domainName, String domain) { + if (domainName != null) { + domain = domainName[0]; + auditTrailSb.append(" domain=" + domain); + if (domain != null) { + // ensure domain starts with '/' and ends with '/' + if (!domain.endsWith("/")) { + domain += '/'; + } + if (!domain.startsWith("/")) { + domain = "/" + domain; + } + } + } + return domain; + } + + @Override + public APIAuthenticationType getAPIType() { + return APIAuthenticationType.PASSWORD_RESET; + } + + @Override + public void setAuthenticators(List authenticators) { + } +} diff --git a/server/src/main/java/com/cloud/user/AccountManager.java b/server/src/main/java/com/cloud/user/AccountManager.java index 72235a808a4..1e5526688b7 100644 --- a/server/src/main/java/com/cloud/user/AccountManager.java +++ b/server/src/main/java/com/cloud/user/AccountManager.java @@ -200,5 +200,7 @@ public interface AccountManager extends AccountService, Configurable { List getApiNameList(); - void checkApiAccess(Account caller, String command); + void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword, boolean skipCurrentPassValidation); + + void checkApiAccess(Account caller, String command); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 6a9e15a58c7..78234497cd0 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1455,7 +1455,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M validateAndUpdateLastNameIfNeeded(updateUserCmd, user); validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); - validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getCurrentPassword()); + validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getCurrentPassword(), false); String email = updateUserCmd.getEmail(); if (StringUtils.isNotBlank(email)) { user.setEmail(email); @@ -1483,7 +1483,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M * * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. */ - protected void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword) { + public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword, boolean skipCurrentPassValidation) { if (newPassword == null) { logger.trace("No new password to update for user: " + user.getUuid()); return; @@ -1498,16 +1498,17 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M boolean isRootAdminExecutingPasswordUpdate = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(callingAccount.getId()); boolean isDomainAdmin = isDomainAdmin(callingAccount.getId()); boolean isAdmin = isDomainAdmin || isRootAdminExecutingPasswordUpdate; + boolean skipValidation = isAdmin || skipCurrentPassValidation; if (isAdmin) { logger.trace(String.format("Admin account [uuid=%s] executing password update for user [%s] ", callingAccount.getUuid(), user.getUuid())); } - if (!isAdmin && StringUtils.isBlank(currentPassword)) { + if (!skipValidation && StringUtils.isBlank(currentPassword)) { throw new InvalidParameterValueException("To set a new password the current password must be provided."); } if (CollectionUtils.isEmpty(_userPasswordEncoders)) { throw new CloudRuntimeException("No user authenticators configured!"); } - if (!isAdmin) { + if (!skipValidation) { validateCurrentPassword(user, currentPassword); } UserAuthenticator userAuthenticator = _userPasswordEncoders.get(0); diff --git a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java new file mode 100644 index 00000000000..a42faf2835a --- /dev/null +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java @@ -0,0 +1,71 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.user; + +import com.cloud.user.UserAccount; +import org.apache.cloudstack.framework.config.ConfigKey; + +public interface UserPasswordResetManager { + ConfigKey UserPasswordResetEnabled = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Boolean.class, + "user.password.reset.enabled", "false", + "Setting this to true allows the ACS user to request an email to reset their password", + false, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetTtl = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class, + "user.password.reset.ttl", "30", + "TTL in minutes for the token generated to reset the ACS user's password", true, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetEmailSender = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "user.password.reset.email.sender", null, + "Sender for emails sent to the user to reset ACS user's password ", true, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "user.password.reset.smtp.host", null, + "Host for SMTP server for sending emails for resetting password for ACS users", + false, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Integer.class, "user.password.reset.smtp.port", "25", + "Port for SMTP server for sending emails for resetting password for ACS users", + false, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPUseAuth = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Boolean.class, "user.password.reset.smtp.useAuth", "false", + "Use auth in the SMTP server for sending emails for resetting password for ACS users", + false, ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPUsername = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "user.password.reset.smtp.username", null, + "Username for SMTP server for sending emails for resetting password for ACS users", + false, ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPPassword = new ConfigKey<>("Secure", String.class, + "user.password.reset.smtp.password", null, + "Password for SMTP server for sending emails for resetting password for ACS users", + false, ConfigKey.Scope.Global); + + void setResetTokenAndSend(UserAccount userAccount); + + boolean validateAndResetPassword(UserAccount user, String token, String password); +} diff --git a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java new file mode 100644 index 00000000000..f35f69fb8bf --- /dev/null +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java @@ -0,0 +1,312 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.user; + +import com.cloud.user.AccountManager; +import com.cloud.user.UserAccount; +import com.cloud.user.UserVO; +import com.cloud.user.dao.UserDao; +import com.cloud.utils.StringUtils; +import com.cloud.utils.component.ManagerBase; +import com.github.mustachejava.DefaultMustacheFactory; +import com.github.mustachejava.Mustache; +import com.github.mustachejava.MustacheFactory; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.resourcedetail.UserDetailVO; +import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; +import org.apache.cloudstack.utils.mailing.MailAddress; +import org.apache.cloudstack.utils.mailing.SMTPMailProperties; +import org.apache.cloudstack.utils.mailing.SMTPMailSender; + +import javax.inject.Inject; +import javax.naming.ConfigurationException; +import java.io.IOException; +import java.io.StringReader; +import java.io.StringWriter; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import static org.apache.cloudstack.config.ApiServiceConfiguration.ManagementServerAddresses; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; + +public class UserPasswordResetManagerImpl extends ManagerBase implements UserPasswordResetManager, Configurable { + + @Inject + private AccountManager accountManager; + + @Inject + private UserDetailsDao userDetailsDao; + + @Inject + private UserDao userDao; + + private SMTPMailSender mailSender; + + public static ConfigKey PasswordResetMailTemplate = + new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class, + "user.password.reset.mail.template", "Hello {{username}}!\n" + + "You have requested to reset your password. Please click the following link to reset your password:\n" + + "http://{{{resetLink}}}\n" + + "If you did not request a password reset, please ignore this email.\n" + + "\n" + + "Regards,\n" + + "The CloudStack Team", + "Password reset mail template. This uses mustache template engine. Available " + + "variables are: username, firstName, lastName, resetLink, token", + true, + ConfigKey.Scope.Global); + + @Override + public String getConfigComponentName() { + return UserPasswordResetManagerImpl.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[]{ + UserPasswordResetEnabled, + UserPasswordResetTtl, + UserPasswordResetEmailSender, + UserPasswordResetSMTPHost, + UserPasswordResetSMTPPort, + UserPasswordResetSMTPUseAuth, + UserPasswordResetSMTPUsername, + UserPasswordResetSMTPPassword, + PasswordResetMailTemplate + }; + } + + @Override + public boolean configure(String name, Map params) throws ConfigurationException { + String smtpHost = UserPasswordResetSMTPHost.value(); + Integer smtpPort = UserPasswordResetSMTPPort.value(); + Boolean useAuth = UserPasswordResetSMTPUseAuth.value(); + String username = UserPasswordResetSMTPUsername.value(); + String password = UserPasswordResetSMTPPassword.value(); + + if (!StringUtils.isEmpty(smtpHost) && smtpPort != null && smtpPort > 0) { + String namespace = "password.reset.smtp"; + + Map configs = new HashMap<>(); + + configs.put(getKey(namespace, SMTPMailSender.CONFIG_HOST), smtpHost); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_PORT), smtpPort.toString()); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_USE_AUTH), useAuth.toString()); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_USERNAME), username); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_PASSWORD), password); + + mailSender = new SMTPMailSender(configs, namespace); + } + return true; + } + + private String getKey(String namespace, String config) { + return String.format("%s.%s", namespace, config); + } + + + protected boolean validateExistingToken(UserAccount userAccount) { + + Map details = userDetailsDao.listDetailsKeyPairs(userAccount.getId()); + + String resetToken = details.get(PasswordResetToken); + String resetTokenExpiryTimeString = details.getOrDefault(PasswordResetTokenExpiryDate, "0"); + + + if (StringUtils.isNotEmpty(resetToken) && StringUtils.isNotEmpty(resetTokenExpiryTimeString)) { + final Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryTimeString)); + final Date currentTime = new Date(); + if (currentTime.after(resetTokenExpiryTime)) { + return true; + } + } else if (StringUtils.isEmpty(resetToken)) { + return true; + } + return false; + } + + public void setResetTokenAndSend(UserAccount userAccount) { + if (mailSender == null) { + logger.debug("Failed to reset token and send email. SMTP mail sender is not configured."); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, + "Failed to reset token and send email. SMTP mail sender is not configured"); + } + + if (!validateExistingToken(userAccount)) { + logger.debug(String.format( + "Failed to reset token and send email. Password reset token is already set for user %s in " + + "domain id: %s with account %s and email %s", + userAccount.getUsername(), userAccount.getDomainId(), + userAccount.getAccountName(), userAccount.getEmail())); + return; + } + + final String resetToken = UUID.randomUUID().toString(); + final Date resetTokenExpiryTime = new Date(System.currentTimeMillis() + UserPasswordResetTtl.value() * 60 * 1000); + + userDetailsDao.addDetail(userAccount.getId(), PasswordResetToken, resetToken, false); + userDetailsDao.addDetail(userAccount.getId(), PasswordResetTokenExpiryDate, String.valueOf(resetTokenExpiryTime.getTime()), false); + + final String email = userAccount.getEmail(); + final String username = userAccount.getUsername(); + final String subject = "Password Reset Request"; + + String resetLink = String.format("%s/client/#/user/resetPassword?username=%s&token=%s", + ManagementServerAddresses.value().split(",")[0], username, resetToken); + String content = getMessageBody(userAccount, resetToken, resetLink); + + SMTPMailProperties mailProperties = new SMTPMailProperties(); + + mailProperties.setSender(new MailAddress(UserPasswordResetEmailSender.value())); + mailProperties.setSubject(subject); + mailProperties.setContent(content); + mailProperties.setContentType("text/html; charset=utf-8"); + + Set addresses = new HashSet<>(); + + addresses.add(new MailAddress(email)); + + mailProperties.setRecipients(addresses); + + mailSender.sendMail(mailProperties); + logger.debug(String.format( + "User password reset email for user id: %d username: %s account id: %d" + + " domain id:%d sent to %s with token expiry at %s", + userAccount.getId(), username, userAccount.getAccountId(), + userAccount.getDomainId(), email, resetTokenExpiryTime)); + } + + @Override + public boolean validateAndResetPassword(UserAccount user, String token, String password) { + UserDetailVO resetTokenDetail = userDetailsDao.findDetail(user.getId(), PasswordResetToken); + UserDetailVO resetTokenExpiryDate = userDetailsDao.findDetail(user.getId(), PasswordResetTokenExpiryDate); + + if (resetTokenDetail == null || resetTokenExpiryDate == null) { + logger.debug(String.format( + "Failed to reset password. No reset token found for user id: %d username: %s account" + + " id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("No reset token found for user %s", user.getUsername())); + } + + Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryDate.getValue())); + + Date now = new Date(); + String resetToken = resetTokenDetail.getValue(); + if (StringUtils.isEmpty(resetToken)) { + logger.debug(String.format( + "Failed to reset password. No reset token found for user id: %d username: %s account" + + " id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("No reset token found for user %s", user.getUsername())); + } + if (!resetToken.equals(token)) { + logger.debug(String.format( + "Failed to reset password. Invalid reset token for user id: %d username: %s " + + "account id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Invalid reset token for user %s", user.getUsername())); + } + if (now.after(resetTokenExpiryTime)) { + logger.debug(String.format( + "Failed to reset password. Reset token has expired for user id: %d username: %s " + + "account id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Reset token has expired for user %s", user.getUsername())); + } + + resetPassword(user, password); + logger.debug(String.format( + "Password reset successful for user id: %d username: %s account id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); + return true; + } + + + void resetPassword(UserAccount userAccount, String password) { + UserVO user = userDao.getUser(userAccount.getId()); + + accountManager.validateUserPasswordAndUpdateIfNeeded(password, user, "", true); + + userDetailsDao.removeDetail(userAccount.getId(), PasswordResetToken); + userDetailsDao.removeDetail(userAccount.getId(), PasswordResetTokenExpiryDate); + + userDao.persist(user); + } + + String getMessageBody(UserAccount userAccount, String token, String resetLink) { + MustacheFactory mf = new DefaultMustacheFactory(); + Mustache mustache = mf.compile(new StringReader(PasswordResetMailTemplate.value()), "password.reset.mail"); + StringWriter writer = new StringWriter(); + + PasswordResetMail values = new PasswordResetMail(userAccount.getUsername(), userAccount.getFirstname(), userAccount.getLastname(), resetLink, token); + + try { + mustache.execute(writer, values).flush(); + } catch (IOException e) { + throw new RuntimeException(e); + } + return writer.toString(); + + } + + static class PasswordResetMail { + private String username; + private String firstName; + private String lastName; + private String resetLink; + private String token; + + + public PasswordResetMail(String username, String firstName, String lastName, String resetLink, String token) { + this.username = username; + this.firstName = firstName; + this.lastName = lastName; + this.resetLink = resetLink; + this.token = token; + } + + public String getUsername() { + return username; + } + + public String getFirstName() { + return firstName; + } + + public String getLastName() { + return lastName; + } + + public String getResetLink() { + return resetLink; + } + + public String getToken() { + return token; + } + } +} diff --git a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml index e9b1cad78d7..1bf921f625e 100644 --- a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml +++ b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml @@ -56,6 +56,7 @@ + diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java index 7b0380f8e64..fed1d95a625 100644 --- a/server/src/test/java/com/cloud/api/ApiServerTest.java +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -16,23 +16,53 @@ // under the License. package com.cloud.api; -import java.util.ArrayList; -import java.util.List; - +import com.cloud.domain.Domain; +import com.cloud.user.UserAccount; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.user.UserPasswordResetManager; +import org.junit.AfterClass; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.MockedConstruction; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; + +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + @RunWith(MockitoJUnitRunner.class) public class ApiServerTest { @InjectMocks ApiServer apiServer = new ApiServer(); + @Mock + UserPasswordResetManager userPasswordResetManager; + + @BeforeClass + public static void beforeClass() throws Exception { + overrideDefaultConfigValue(UserPasswordResetEnabled, "_value", true); + } + + @AfterClass + public static void afterClass() throws Exception { + overrideDefaultConfigValue(UserPasswordResetEnabled, "_value", false); + } + + private static void overrideDefaultConfigValue(final ConfigKey configKey, final String name, final Object o) throws IllegalAccessException, NoSuchFieldException { + Field f = ConfigKey.class.getDeclaredField(name); + f.setAccessible(true); + f.set(configKey, o); + } + private void runTestSetupIntegrationPortListenerInvalidPorts(Integer port) { try (MockedConstruction mocked = Mockito.mockConstruction(ApiServer.ListenerThread.class)) { @@ -61,4 +91,60 @@ public class ApiServerTest { Mockito.verify(listenerThread).start(); } } + + @Test + public void testForgotPasswordSuccess() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); + Mockito.when(domain.getState()).thenReturn(Domain.State.Active); + Mockito.doNothing().when(userPasswordResetManager).setResetTokenAndSend(userAccount); + Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); + Mockito.verify(userPasswordResetManager).setResetTokenAndSend(userAccount); + } + + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureNoEmail() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getEmail()).thenReturn(""); + apiServer.forgotPassword(userAccount, domain); + } + + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureDisabledUser() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.when(userAccount.getState()).thenReturn("DISABLED"); + apiServer.forgotPassword(userAccount, domain); + } + + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureDisabledAccount() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("DISABLED"); + apiServer.forgotPassword(userAccount, domain); + } + + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureInactiveDomain() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); + Mockito.when(domain.getState()).thenReturn(Domain.State.Inactive); + apiServer.forgotPassword(userAccount, domain); + } } diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index db4fbed5320..9daa19206fa 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -405,7 +405,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.doNothing().when(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock); Mockito.doNothing().when(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock, userVoMock); Mockito.doNothing().when(accountManagerImpl).validateAndUpdateUsernameIfNeeded(UpdateUserCmdMock, userVoMock, accountMock); - Mockito.doNothing().when(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(Mockito.anyString(), Mockito.eq(userVoMock), Mockito.anyString()); + Mockito.doNothing().when(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(Mockito.anyString(), Mockito.eq(userVoMock), Mockito.anyString(), Mockito.eq(false)); Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock)); Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDaoMock).findById(Mockito.anyLong()); @@ -421,7 +421,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { inOrder.verify(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock); inOrder.verify(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock, userVoMock); inOrder.verify(accountManagerImpl).validateAndUpdateUsernameIfNeeded(UpdateUserCmdMock, userVoMock, accountMock); - inOrder.verify(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(UpdateUserCmdMock.getPassword(), userVoMock, UpdateUserCmdMock.getCurrentPassword()); + inOrder.verify(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(UpdateUserCmdMock.getPassword(), userVoMock, UpdateUserCmdMock.getCurrentPassword(), false); inOrder.verify(userVoMock, Mockito.times(numberOfExpectedCallsForSetEmailAndSetTimeZone)).setEmail(Mockito.anyString()); inOrder.verify(userVoMock, Mockito.times(numberOfExpectedCallsForSetEmailAndSetTimeZone)).setTimezone(Mockito.anyString()); @@ -707,14 +707,14 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test public void valiateUserPasswordAndUpdateIfNeededTestPasswordNull() { - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(null, userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(null, userVoMock, null, false); Mockito.verify(userVoMock, Mockito.times(0)).setPassword(Mockito.anyString()); } @Test(expected = InvalidParameterValueException.class) public void valiateUserPasswordAndUpdateIfNeededTestBlankPassword() { - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(" ", userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(" ", userVoMock, null, false); } @Test(expected = InvalidParameterValueException.class) @@ -728,7 +728,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, " "); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, " ", false); } @Test(expected = CloudRuntimeException.class) @@ -743,7 +743,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, null, false); } @Test @@ -762,7 +762,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); @@ -784,7 +784,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); @@ -807,7 +807,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword, false); Mockito.verify(accountManagerImpl, Mockito.times(1)).validateCurrentPassword(userVoMock, currentPassword); Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); @@ -826,7 +826,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.doThrow(new InvalidParameterValueException("")).when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword, false); } private String configureUserMockAuthenticators(String newPassword) { diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index b4c2dafd664..4cf7413f3f3 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -487,4 +487,8 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco public List getApiNameList() { return null; } + + @Override + public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword, boolean skipCurrentPassValidation) { + } } diff --git a/server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java new file mode 100644 index 00000000000..17092e6311d --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java @@ -0,0 +1,150 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.user; + +import com.cloud.user.UserAccount; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.resourcedetail.UserDetailVO; +import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.Collections; +import java.util.Map; + +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; + +@RunWith(MockitoJUnitRunner.class) +public class UserPasswordResetManagerImplTest { + @Spy + @InjectMocks + UserPasswordResetManagerImpl passwordReset; + + @Mock + private UserDetailsDao userDetailsDao; + + @Test + public void testGetMessageBody() { + ConfigKey passwordResetMailTemplate = Mockito.mock(ConfigKey.class); + UserPasswordResetManagerImpl.PasswordResetMailTemplate = passwordResetMailTemplate; + Mockito.when(passwordResetMailTemplate.value()).thenReturn("Hello {{username}}!\n" + + "You have requested to reset your password. Please click the following link to reset your password:\n" + + "{{{resetLink}}}\n" + + "If you did not request a password reset, please ignore this email.\n" + + "\n" + + "Regards,\n" + + "The CloudStack Team"); + + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getUsername()).thenReturn("test_user"); + + String messageBody = passwordReset.getMessageBody(userAccount, "reset_token", "reset_link"); + String expectedMessageBody = "Hello test_user!\n" + + "You have requested to reset your password. Please click the following link to reset your password:\n" + + "reset_link\n" + + "If you did not request a password reset, please ignore this email.\n" + + "\n" + + "Regards,\n" + + "The CloudStack Team"; + Assert.assertEquals("Message body doesn't match", expectedMessageBody, messageBody); + } + + @Test + public void testValidateAndResetPassword() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userAccount.getUsername()).thenReturn("test_user"); + + Mockito.doNothing().when(passwordReset).resetPassword(userAccount, "new_password"); + + UserDetailVO resetTokenDetail = Mockito.mock(UserDetailVO.class); + UserDetailVO resetTokenExpiryDate = Mockito.mock(UserDetailVO.class); + Mockito.when(userDetailsDao.findDetail(1L, PasswordResetToken)).thenReturn(resetTokenDetail); + Mockito.when(userDetailsDao.findDetail(1L, PasswordResetTokenExpiryDate)).thenReturn(resetTokenExpiryDate); + Mockito.when(resetTokenExpiryDate.getValue()).thenReturn(String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000)); + + try { + passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password"); + Assert.fail("Should have thrown exception"); + } catch (ServerApiException e) { + Assert.assertEquals("No reset token found for user test_user", e.getMessage()); + } + + Mockito.when(resetTokenDetail.getValue()).thenReturn("reset_token_XXX"); + + try { + passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password"); + Assert.fail("Should have thrown exception"); + } catch (ServerApiException e) { + Assert.assertEquals("Invalid reset token for user test_user", e.getMessage()); + } + + Mockito.when(resetTokenDetail.getValue()).thenReturn("reset_token"); + + try { + passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password"); + Assert.fail("Should have thrown exception"); + } catch (ServerApiException e) { + Assert.assertEquals("Reset token has expired for user test_user", e.getMessage()); + } + + Mockito.when(resetTokenExpiryDate.getValue()).thenReturn(String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000)); + + Assert.assertTrue(passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password")); + Mockito.verify(passwordReset, Mockito.times(1)).resetPassword(userAccount, "new_password"); + } + + @Test + public void testValidateExistingTokenFirstRequest() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Collections.emptyMap()); + + Assert.assertTrue(passwordReset.validateExistingToken(userAccount)); + } + + @Test + public void testValidateExistingTokenSecondRequestExpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Map.of( + PasswordResetToken, "reset_token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000))); + + Assert.assertTrue(passwordReset.validateExistingToken(userAccount)); + } + + + @Test + public void testValidateExistingTokenSecondRequestUnexpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Map.of( + PasswordResetToken, "reset_token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000))); + + Assert.assertFalse(passwordReset.validateExistingToken(userAccount)); + } +} diff --git a/tools/apidoc/gen_toc.py b/tools/apidoc/gen_toc.py index ace0dbb33f3..aea803035ce 100644 --- a/tools/apidoc/gen_toc.py +++ b/tools/apidoc/gen_toc.py @@ -282,12 +282,14 @@ known_categories = { 'Webhook': 'Webhook', 'Webhooks': 'Webhook', 'purgeExpungedResources': 'Resource', + 'forgotPassword': 'Authentication', + 'resetPassword': 'Authentication', 'BgpPeer': 'BGP Peer', 'createASNRange': 'AS Number Range', 'listASNRange': 'AS Number Range', 'deleteASNRange': 'AS Number Range', 'listASNumbers': 'AS Number', - 'releaseASNumber': 'AS Number' + 'releaseASNumber': 'AS Number', } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 7336c038c89..f709c73e51a 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -417,6 +417,7 @@ "label.availableprocessors": "Available processor cores", "label.availablevirtualmachinecount": "Available Instances", "label.back": "Back", +"label.back.login": "Back to login", "label.backup": "Backups", "label.backup.attach.restore": "Restore and attach backup volume", "label.backup.configure.schedule": "Configure Backup Schedule", @@ -1002,6 +1003,7 @@ "label.force.reboot": "Force reboot", "label.forceencap": "Force UDP encapsulation of ESP packets", "label.forgedtransmits": "Forged transmits", +"label.forgot.password": "Forgot password?", "label.format": "Format", "label.fornsx": "NSX", "label.forvpc": "VPC", @@ -3174,6 +3176,7 @@ "message.failed.to.add": "Failed to add", "message.failed.to.assign.vms": "Failed to assign Instances", "message.failed.to.remove": "Failed to remove", +"message.forgot.password.success": "An email has been sent to your email address with instructions on how to reset your password.", "message.generate.keys": "Please confirm that you would like to generate new API/Secret keys for this User.", "message.chart.statistic.info": "The shown charts are self-adjustable, that means, if the value gets close to the limit or overpass it, it will grow to adjust the shown value", "message.chart.statistic.info.hypervisor.additionals": "The metrics data depend on the hypervisor plugin used for each hypervisor. The behavior can vary across different hypervisors. For instance, with KVM, metrics are real-time statistics provided by libvirt. In contrast, with VMware, the metrics are averaged data for a given time interval controlled by configuration.", @@ -3301,6 +3304,8 @@ "message.offering.internet.protocol.warning": "WARNING: IPv6 supported Networks use static routing and will require upstream routes to be configured manually.", "message.offering.ipv6.warning": "Please refer documentation for creating IPv6 enabled Network/VPC offering IPv6 support in CloudStack - Isolated Networks and VPC Network Tiers", "message.ovf.configurations": "OVF configurations available for the selected appliance. Please select the desired value. Incompatible compute offerings will get disabled.", +"message.password.reset.failed": "Failed to reset password.", +"message.password.reset.success": "Password has been reset successfully. Please login using your new credentials.", "message.path": "Path : ", "message.path.description": "NFS: exported path from the server. VMFS: /datacenter name/datastore name. SharedMountPoint: path where primary storage is mounted, such as /mnt/primary.", "message.please.confirm.remove.ssh.key.pair": "Please confirm that you want to remove this SSH key pair.", diff --git a/ui/src/config/router.js b/ui/src/config/router.js index 0d0783a0906..16599a0c367 100644 --- a/ui/src/config/router.js +++ b/ui/src/config/router.js @@ -300,6 +300,16 @@ export const constantRouterMap = [ path: 'login', name: 'login', component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/Login') + }, + { + path: 'forgotPassword', + name: 'forgotPassword', + component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/ForgotPassword') + }, + { + path: 'resetPassword', + name: 'resetPassword', + component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/ResetPassword') } ] }, diff --git a/ui/src/permission.js b/ui/src/permission.js index 4380c7660d8..266dc992c8d 100644 --- a/ui/src/permission.js +++ b/ui/src/permission.js @@ -30,7 +30,7 @@ import { ACCESS_TOKEN, APIS, SERVER_MANAGER, CURRENT_PROJECT } from '@/store/mut NProgress.configure({ showSpinner: false }) // NProgress Configuration -const allowList = ['login', 'VerifyOauth'] // no redirect allowlist +const allowList = ['login', 'VerifyOauth', 'forgotPassword', 'resetPassword'] // no redirect allowlist router.beforeEach((to, from, next) => { // start progress bar diff --git a/ui/src/utils/request.js b/ui/src/utils/request.js index c2fe04ab9d1..7c757691f2b 100644 --- a/ui/src/utils/request.js +++ b/ui/src/utils/request.js @@ -51,7 +51,7 @@ const err = (error) => { }) } if (response.status === 401) { - if (response.config && response.config.params && ['listIdps', 'cloudianIsEnabled'].includes(response.config.params.command)) { + if (response.config && response.config.params && ['forgotPassword', 'listIdps', 'cloudianIsEnabled'].includes(response.config.params.command)) { return } const originalPath = router.currentRoute.value.fullPath diff --git a/ui/src/views/auth/ForgotPassword.vue b/ui/src/views/auth/ForgotPassword.vue new file mode 100644 index 00000000000..2d45938417f --- /dev/null +++ b/ui/src/views/auth/ForgotPassword.vue @@ -0,0 +1,260 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + + + + diff --git a/ui/src/views/auth/Login.vue b/ui/src/views/auth/Login.vue index 8503f71082b..13645565557 100644 --- a/ui/src/views/auth/Login.vue +++ b/ui/src/views/auth/Login.vue @@ -152,7 +152,16 @@ @click="handleSubmit" >{{ $t('label.login') }} - + + + + + + + {{ $t('label.forgot.password') }} + + +

or

@@ -220,7 +229,8 @@ export default { loginBtn: false, loginType: 0 }, - server: '' + server: '', + forgotPasswordEnabled: false } }, created () { @@ -303,6 +313,15 @@ export default { }) } }) + api('forgotPassword', {}).then(response => { + this.forgotPasswordEnabled = response.forgotpasswordresponse.enabled + }).catch((err) => { + if (err?.response?.data === null) { + this.forgotPasswordEnabled = true + } else { + this.forgotPasswordEnabled = false + } + }) }, // handler async handleUsernameOrEmail (rule, value) { diff --git a/ui/src/views/auth/ResetPassword.vue b/ui/src/views/auth/ResetPassword.vue new file mode 100644 index 00000000000..8a9047c5d3e --- /dev/null +++ b/ui/src/views/auth/ResetPassword.vue @@ -0,0 +1,318 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + + + + diff --git a/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java b/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java index 4afa3c9100b..b354772fde0 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java @@ -48,16 +48,16 @@ public class SMTPMailSender { protected Session session = null; protected SMTPSessionProperties sessionProps; - protected static final String CONFIG_HOST = "host"; - protected static final String CONFIG_PORT = "port"; - protected static final String CONFIG_USE_AUTH = "useAuth"; - protected static final String CONFIG_USERNAME = "username"; - protected static final String CONFIG_PASSWORD = "password"; - protected static final String CONFIG_DEBUG_MODE = "debug"; - protected static final String CONFIG_USE_STARTTLS = "useStartTLS"; - protected static final String CONFIG_ENABLED_SECURITY_PROTOCOLS = "enabledSecurityProtocols"; - protected static final String CONFIG_TIMEOUT = "timeout"; - protected static final String CONFIG_CONNECTION_TIMEOUT = "connectiontimeout"; + public static final String CONFIG_HOST = "host"; + public static final String CONFIG_PORT = "port"; + public static final String CONFIG_USE_AUTH = "useAuth"; + public static final String CONFIG_USERNAME = "username"; + public static final String CONFIG_PASSWORD = "password"; + public static final String CONFIG_DEBUG_MODE = "debug"; + public static final String CONFIG_USE_STARTTLS = "useStartTLS"; + public static final String CONFIG_ENABLED_SECURITY_PROTOCOLS = "enabledSecurityProtocols"; + public static final String CONFIG_TIMEOUT = "timeout"; + public static final String CONFIG_CONNECTION_TIMEOUT = "connectiontimeout"; protected Map configs; protected String namespace;