diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 133e2c35b4e..4d083fdd7e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,7 @@ jobs: fail-fast: false matrix: tests: [ "smoke/test_accounts + smoke/test_account_access smoke/test_affinity_groups smoke/test_affinity_groups_projects smoke/test_annotations diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ClusterResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ClusterResponse.java index 72dab3da3b1..ca01a2012f6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ClusterResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ClusterResponse.java @@ -73,7 +73,7 @@ public class ClusterResponse extends BaseResponseWithAnnotations { @SerializedName("capacity") @Param(description = "the capacity of the Cluster", responseObject = CapacityResponse.class) - private List capacitites; + private List capacities; @SerializedName("cpuovercommitratio") @Param(description = "The cpu overcommit ratio of the cluster") @@ -171,12 +171,12 @@ public class ClusterResponse extends BaseResponseWithAnnotations { this.managedState = managedState; } - public List getCapacitites() { - return capacitites; + public List getCapacities() { + return capacities; } - public void setCapacitites(ArrayList arrayList) { - this.capacitites = arrayList; + public void setCapacities(ArrayList arrayList) { + this.capacities = arrayList; } public void setCpuOvercommitRatio(String cpuovercommitratio) { @@ -219,4 +219,32 @@ public class ClusterResponse extends BaseResponseWithAnnotations { public Map getResourceDetails() { return resourceDetails; } + + public String getCpuovercommitratio() { + return cpuovercommitratio; + } + + public void setCpuovercommitratio(String cpuovercommitratio) { + this.cpuovercommitratio = cpuovercommitratio; + } + + public String getMemoryovercommitratio() { + return memoryovercommitratio; + } + + public void setMemoryovercommitratio(String memoryovercommitratio) { + this.memoryovercommitratio = memoryovercommitratio; + } + + public String getOvm3vip() { + return ovm3vip; + } + + public void setOvm3vip(String ovm3vip) { + this.ovm3vip = ovm3vip; + } + + public void setCapacities(List capacities) { + this.capacities = capacities; + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/HostResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/HostResponse.java index 8d98b6573f4..274c1fd43c1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/HostResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/HostResponse.java @@ -280,7 +280,7 @@ public class HostResponse extends BaseResponseWithAnnotations { @SerializedName("ueficapability") @Param(description = "true if the host has capability to support UEFI boot") - private Boolean uefiCapabilty; + private Boolean uefiCapability; @SerializedName(ApiConstants.ENCRYPTION_SUPPORTED) @Param(description = "true if the host supports encryption", since = "4.18") @@ -735,7 +735,7 @@ public class HostResponse extends BaseResponseWithAnnotations { return clusterType; } - public Boolean isLocalStorageActive() { + public Boolean getLocalStorageActive() { return localStorageActive; } @@ -755,7 +755,7 @@ public class HostResponse extends BaseResponseWithAnnotations { return hasEnoughCapacity; } - public Boolean isSuitableForMigration() { + public Boolean getSuitableForMigration() { return suitableForMigration; } @@ -767,8 +767,8 @@ public class HostResponse extends BaseResponseWithAnnotations { return haHost; } - public void setUefiCapabilty(Boolean hostCapability) { - this.uefiCapabilty = hostCapability; + public void setUefiCapability(Boolean hostCapability) { + this.uefiCapability = hostCapability; } public void setEncryptionSupported(Boolean encryptionSupported) { @@ -786,4 +786,76 @@ public class HostResponse extends BaseResponseWithAnnotations { public void setIsTagARule(Boolean tagARule) { isTagARule = tagARule; } + + public Long getCpuAllocatedValue() { + return cpuAllocatedValue; + } + + public String getCpuAllocatedPercentage() { + return cpuAllocatedPercentage; + } + + public String getCpuAllocatedWithOverprovisioning() { + return cpuAllocatedWithOverprovisioning; + } + + public Double getCpuloadaverage() { + return cpuloadaverage; + } + + public void setCpuloadaverage(Double cpuloadaverage) { + this.cpuloadaverage = cpuloadaverage; + } + + public String getMemWithOverprovisioning() { + return memWithOverprovisioning; + } + + public String getMemoryAllocatedPercentage() { + return memoryAllocatedPercentage; + } + + public Long getMemoryAllocatedBytes() { + return memoryAllocatedBytes; + } + + public Boolean getTagARule() { + return isTagARule; + } + + public void setTagARule(Boolean tagARule) { + isTagARule = tagARule; + } + + public Boolean getHasEnoughCapacity() { + return hasEnoughCapacity; + } + + public void setDetails(Map details) { + this.details = details; + } + + public String getAnnotation() { + return annotation; + } + + public Date getLastAnnotated() { + return lastAnnotated; + } + + public String getUsername() { + return username; + } + + public Boolean getUefiCapability() { + return uefiCapability; + } + + public Boolean getEncryptionSupported() { + return encryptionSupported; + } + + public Boolean getInstanceConversionSupported() { + return instanceConversionSupported; + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ManagementServerResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ManagementServerResponse.java index 330f91e69f3..a471045eb67 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ManagementServerResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ManagementServerResponse.java @@ -167,4 +167,8 @@ public class ManagementServerResponse extends BaseResponse { public void setServiceIp(String serviceIp) { this.serviceIp = serviceIp; } + + public String getKernelVersion() { + return kernelVersion; + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java index 9e7f5159e0e..bd468a9201f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java @@ -371,4 +371,16 @@ public class StoragePoolResponse extends BaseResponseWithAnnotations { public void setNfsMountOpts(String nfsMountOpts) { this.nfsMountOpts = nfsMountOpts; } + + public Long getAllocatedIops() { + return allocatedIops; + } + + public Boolean getTagARule() { + return isTagARule; + } + + public void setTagARule(Boolean tagARule) { + isTagARule = tagARule; + } } 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 8deae7d80d3..8a259225578 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 @@ -1133,4 +1133,8 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co } this.vnfDetails.put(key,value); } + + public void setIpAddress(String ipAddress) { + this.ipAddress = ipAddress; + } } 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 726c9adf8a3..f3683473e45 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 @@ -211,7 +211,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co @SerializedName("destroyed") @Param(description = "the boolean state of whether the volume is destroyed or not") - private Boolean destroyed; + private boolean destroyed; @SerializedName(ApiConstants.SERVICE_OFFERING_ID) @Param(description = "ID of the service offering for root disk") @@ -227,7 +227,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co @SerializedName("isextractable") @Param(description = "true if the volume is extractable, false otherwise") - private Boolean extractable; + private boolean extractable; @SerializedName(ApiConstants.STATUS) @Param(description = "the status of the volume") @@ -235,7 +235,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co @SerializedName(ApiConstants.DISPLAY_VOLUME) @Param(description = "an optional field whether to the display the volume to the end user or not.", authorized = {RoleType.Admin}) - private Boolean displayVolume; + private boolean displayVolume; @SerializedName(ApiConstants.PATH) @Param(description = "the path of the volume") @@ -318,11 +318,11 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co return this.getId(); } - public Boolean isDestroyed() { + public boolean isDestroyed() { return destroyed; } - public void setDestroyed(Boolean destroyed) { + public void setDestroyed(boolean destroyed) { this.destroyed = destroyed; } @@ -521,7 +521,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co this.serviceOfferingDisplayText = serviceOfferingDisplayText; } - public void setExtractable(Boolean extractable) { + public void setExtractable(boolean extractable) { this.extractable = extractable; } @@ -539,7 +539,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co this.projectName = projectName; } - public void setDisplayVolume(Boolean displayVm) { + public void setDisplayVolume(boolean displayVm) { this.displayVolume = displayVm; } @@ -755,7 +755,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co return serviceOfferingDisplayText; } - public Boolean getExtractable() { + public boolean isExtractable() { return extractable; } @@ -763,7 +763,7 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co return status; } - public Boolean getDisplayVolume() { + public boolean isDisplayVolume() { return displayVolume; } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ZoneResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ZoneResponse.java index a898cd9d577..0d76fd2d3f9 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ZoneResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ZoneResponse.java @@ -95,7 +95,7 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso @SerializedName("securitygroupsenabled") @Param(description = "true if security groups support is enabled, false otherwise") - private Boolean securityGroupsEnabled; + private boolean securityGroupsEnabled; @SerializedName("allocationstate") @Param(description = "the allocation state of the cluster") @@ -111,11 +111,11 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso @SerializedName("capacity") @Param(description = "the capacity of the Zone", responseObject = CapacityResponse.class) - private List capacitites; + private List capacities; @SerializedName(ApiConstants.LOCAL_STORAGE_ENABLED) @Param(description = "true if local storage offering enabled, false otherwise") - private Boolean localStorageEnabled; + private boolean localStorageEnabled; @SerializedName(ApiConstants.TAGS) @Param(description = "the list of resource tags associated with zone.", responseObject = ResourceTagResponse.class, since = "4.3") @@ -201,7 +201,7 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso this.networkType = networkType; } - public void setSecurityGroupsEnabled(Boolean securityGroupsEnabled) { + public void setSecurityGroupsEnabled(boolean securityGroupsEnabled) { this.securityGroupsEnabled = securityGroupsEnabled; } @@ -217,15 +217,15 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso this.dhcpProvider = dhcpProvider; } - public void setCapacitites(List capacitites) { - this.capacitites = capacitites; + public void setCapacities(List capacities) { + this.capacities = capacities; } public void setDomainName(String domainName) { this.domainName = domainName; } - public void setLocalStorageEnabled(Boolean localStorageEnabled) { + public void setLocalStorageEnabled(boolean localStorageEnabled) { this.localStorageEnabled = localStorageEnabled; } @@ -328,8 +328,8 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso return dhcpProvider; } - public List getCapacitites() { - return capacitites; + public List getCapacities() { + return capacities; } public boolean isLocalStorageEnabled() { @@ -344,6 +344,18 @@ public class ZoneResponse extends BaseResponseWithAnnotations implements SetReso return resourceDetails; } + public Boolean getAllowUserSpecifyVRMtu() { + return allowUserSpecifyVRMtu; + } + + public Integer getRouterPrivateInterfaceMaxMtu() { + return routerPrivateInterfaceMaxMtu; + } + + public Integer getRouterPublicInterfaceMaxMtu() { + return routerPublicInterfaceMaxMtu; + } + @Override public void setResourceIconResponse(ResourceIconResponse resourceIconResponse) { this.resourceIconResponse = resourceIconResponse; diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java index 7e902bc61fe..afc8be1e5f9 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java @@ -100,6 +100,8 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase { @Inject SnapshotZoneDao snapshotZoneDao; + private final List snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error); + public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) { List snaps = snapshotStoreDao.listReadyBySnapshot(snapshotId, DataStoreRole.Image); for (SnapshotDataStoreVO ref : snaps) { @@ -197,9 +199,8 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase { boolean result = false; boolean resultIsSet = false; - final List snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.BackedUp, Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error); try { - while (snapshot != null && snapshotStatesAbleToDeleteSnapshot.contains(snapshot.getState())) { + do { SnapshotInfo child = snapshot.getChild(); if (child != null) { @@ -245,7 +246,7 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase { } snapshot = parent; - } + } while (snapshot != null && snapshotStatesAbleToDeleteSnapshot.contains(snapshot.getState())); } catch (Exception e) { logger.error(String.format("Failed to delete snapshot [%s] on storage [%s] due to [%s].", snapshotTo, storageToString, e.getMessage()), e); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index 151344575dc..317e7f13517 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -1626,8 +1626,7 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne private void updateNodeCount(KubernetesClusterVO kubernetesCluster) { List nodeList = kubernetesClusterVmMapDao.listByClusterId(kubernetesCluster.getId()); kubernetesCluster.setControlNodeCount(nodeList.stream().filter(KubernetesClusterVmMapVO::isControlNode).count()); - kubernetesCluster.setNodeCount(nodeList.size()); - kubernetesCluster.setNodeCount(nodeList.size()); + kubernetesCluster.setNodeCount(nodeList.size() - kubernetesCluster.getControlNodeCount()); kubernetesClusterDao.persist(kubernetesCluster); } diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 729c7a9e43a..b81a3e57ad0 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -206,7 +206,7 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { return true; } else if (entity instanceof Network && accessType != null && accessType == AccessType.UseEntry) { - _networkMgr.checkNetworkPermissions(caller, (Network)entity); + _networkMgr.checkNetworkPermissions(caller, (Network) entity); } else if (entity instanceof Network && accessType != null && accessType == AccessType.OperateEntry) { _networkMgr.checkNetworkOperatePermissions(caller, (Network)entity); } else if (entity instanceof VirtualRouter) { @@ -214,30 +214,58 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { } else if (entity instanceof AffinityGroup) { return false; } else { - if (_accountService.isNormalUser(caller.getId())) { - Account account = _accountDao.findById(entity.getAccountId()); - String errorMessage = String.format("%s does not have permission to operate with resource", caller); - if (account != null && account.getType() == Account.Type.PROJECT) { - //only project owner can delete/modify the project - if (accessType != null && accessType == AccessType.ModifyProject) { - if (!_projectMgr.canModifyProjectAccount(caller, account.getId())) { - throw new PermissionDeniedException(errorMessage); - } - } else if (!_projectMgr.canAccessProjectAccount(caller, account.getId())) { - throw new PermissionDeniedException(errorMessage); - } - checkOperationPermitted(caller, entity); - } else { - if (caller.getId() != entity.getAccountId()) { - throw new PermissionDeniedException(errorMessage); - } - } - } + validateCallerHasAccessToEntityOwner(caller, entity, accessType); } return true; } - private boolean checkOperationPermitted(Account caller, ControlledEntity entity) { + protected void validateCallerHasAccessToEntityOwner(Account caller, ControlledEntity entity, AccessType accessType) { + PermissionDeniedException exception = new PermissionDeniedException("Caller does not have permission to operate with provided resource."); + String entityLog = String.format("entity [owner ID: %d, type: %s]", entity.getAccountId(), + entity.getEntityType().getSimpleName()); + + if (_accountService.isRootAdmin(caller.getId())) { + return; + } + + if (caller.getId() == entity.getAccountId()) { + return; + } + + Account owner = _accountDao.findById(entity.getAccountId()); + if (owner == null) { + s_logger.error(String.format("Owner not found for %s", entityLog)); + throw exception; + } + + Account.Type callerAccountType = caller.getType(); + if ((callerAccountType == Account.Type.DOMAIN_ADMIN || callerAccountType == Account.Type.RESOURCE_DOMAIN_ADMIN) && + _domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) { + return; + } + + if (owner.getType() == Account.Type.PROJECT) { + // only project owner can delete/modify the project + if (accessType == AccessType.ModifyProject) { + if (!_projectMgr.canModifyProjectAccount(caller, owner.getId())) { + s_logger.error(String.format("Caller ID: %d does not have permission to modify project with " + + "owner ID: %d", caller.getId(), owner.getId())); + throw exception; + } + } else if (!_projectMgr.canAccessProjectAccount(caller, owner.getId())) { + s_logger.error(String.format("Caller ID: %d does not have permission to access project with " + + "owner ID: %d", caller.getId(), owner.getId())); + throw exception; + } + checkOperationPermitted(caller, entity); + return; + } + + s_logger.error(String.format("Caller ID: %d does not have permission to access %s", caller.getId(), entityLog)); + throw exception; + } + + protected boolean checkOperationPermitted(Account caller, ControlledEntity entity) { User user = CallContext.current().getCallingUser(); Project project = projectDao.findByProjectAccountId(entity.getAccountId()); if (project == null) { diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index d247aaf5328..cd1d65366ad 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -1525,7 +1525,7 @@ public class ApiResponseHelper implements ResponseGenerator { } // Do it for stats as well. capacityResponses.addAll(getStatsCapacityresponse(null, cluster.getId(), pod.getId(), pod.getDataCenterId())); - clusterResponse.setCapacitites(new ArrayList(capacityResponses)); + clusterResponse.setCapacities(new ArrayList(capacityResponses)); } clusterResponse.setHasAnnotation(annotationDao.hasAnnotations(cluster.getUuid(), AnnotationService.EntityType.CLUSTER.name(), _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); diff --git a/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java index 2bfbb3b9d67..902c9c9bb25 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java @@ -90,7 +90,7 @@ public class DataCenterJoinDaoImpl extends GenericDaoBase implements Map hostDetails = hostDetailsDao.findDetails(host.getId()); if (hostDetails != null) { if (hostDetails.containsKey(Host.HOST_UEFI_ENABLE)) { - hostResponse.setUefiCapabilty(Boolean.parseBoolean((String) hostDetails.get(Host.HOST_UEFI_ENABLE))); + hostResponse.setUefiCapability(Boolean.parseBoolean((String) hostDetails.get(Host.HOST_UEFI_ENABLE))); } else { - hostResponse.setUefiCapabilty(new Boolean(false)); + hostResponse.setUefiCapability(new Boolean(false)); } } if (details.contains(HostDetails.all) && (host.getHypervisorType() == Hypervisor.HypervisorType.KVM || diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index dd788aee5d2..3d392b00570 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -37,6 +37,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -2280,9 +2281,6 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C Long associatedNetworkId = cmd.getAssociatedNetworkId(); String networkFilterStr = cmd.getNetworkFilter(); - boolean applyManualPagination = CollectionUtils.isNotEmpty(supportedServicesStr) || - Boolean.TRUE.equals(canUseForDeploy); - String vlanId = null; if (cmd instanceof ListNetworksCmdByAdmin) { vlanId = ((ListNetworksCmdByAdmin)cmd).getVlan(); @@ -2368,13 +2366,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C isRecursive = true; } - Long offset = cmd.getStartIndex(); - Long limit = cmd.getPageSizeVal(); - if (applyManualPagination) { - offset = null; - limit = null; - } - Filter searchFilter = new Filter(NetworkVO.class, "id", false, offset, limit); + Filter searchFilter = new Filter(NetworkVO.class, "id", false, null, null); SearchBuilder sb = _networksDao.createSearchBuilder(); if (forVpc != null) { @@ -2429,123 +2421,113 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C sb.join("associatedNetworkSearch", associatedNetworkSearch, sb.entity().getId(), associatedNetworkSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER); } - SearchCriteria mainSearchCriteria = createNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, - guestIpType, trafficType, physicalNetworkId, networkOfferingId, aclType, restartRequired, - specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId); - SearchCriteria additionalSearchCriteria = _networksDao.createSearchCriteria(); + List networksToReturn = new ArrayList(); if (isSystem == null || !isSystem) { if (!permittedAccounts.isEmpty()) { if (Arrays.asList(Network.NetworkFilter.Account, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) { //get account level networks - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, - getAccountSpecificNetworksSearchCriteria(sb, permittedAccounts, skipProjectNetworks)); + networksToReturn.addAll(listAccountSpecificNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, permittedAccounts)); } if (domainId != null && Arrays.asList(Network.NetworkFilter.Domain, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) { //get domain level networks - SearchCriteria domainLevelSC = getDomainLevelNetworksSearchCriteria(sb, domainId, false); - if (domainLevelSC != null) { - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, domainLevelSC); - } + networksToReturn.addAll(listDomainLevelNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, domainId, false)); } if (Arrays.asList(Network.NetworkFilter.Shared, Network.NetworkFilter.All).contains(networkFilter)) { // get shared networks - SearchCriteria sharedNetworksSC = getSharedNetworksSearchCriteria(sb, permittedAccounts); - if (sharedNetworksSC != null) { - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, sharedNetworksSC); - } + List sharedNetworks = listSharedNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, permittedAccounts); + addNetworksToReturnIfNotExist(networksToReturn, sharedNetworks); + } } else { if (Arrays.asList(Network.NetworkFilter.Account, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) { //add account specific networks - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, - getAccountSpecificNetworksByDomainPathSearchCriteria(sb, path, isRecursive, - skipProjectNetworks)); + networksToReturn.addAll(listAccountSpecificNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive)); } if (Arrays.asList(Network.NetworkFilter.Domain, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) { //add domain specific networks of domain + parent domains - SearchCriteria domainSpecificNetworksByDomainPathSC = - getDomainSpecificNetworksByDomainPathSearchCriteria(sb, path, isRecursive); - if (domainSpecificNetworksByDomainPathSC != null) { - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, domainSpecificNetworksByDomainPathSC); - } + networksToReturn.addAll(listDomainSpecificNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive)); //add networks of subdomains if (domainId == null) { - SearchCriteria domainLevelSC = getDomainLevelNetworksSearchCriteria(sb, caller.getDomainId(), true); - if (domainLevelSC != null) { - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, domainLevelSC); - } + networksToReturn.addAll(listDomainLevelNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, caller.getDomainId(), true)); } } if (Arrays.asList(Network.NetworkFilter.Shared, Network.NetworkFilter.All).contains(networkFilter)) { // get shared networks - SearchCriteria sharedNetworksSC = getSharedNetworksByDomainPathSearchCriteria(sb, path, isRecursive); - if (sharedNetworksSC != null) { - additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC, sharedNetworksSC); - } + List sharedNetworks = listSharedNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive); + addNetworksToReturnIfNotExist(networksToReturn, sharedNetworks); } } - if (CollectionUtils.isNotEmpty(additionalSearchCriteria.getValues())) { - mainSearchCriteria.addAnd("id", SearchCriteria.Op.SC, additionalSearchCriteria); - } } else { - if (skipProjectNetworks) { - mainSearchCriteria.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); - } else { - mainSearchCriteria.setJoinParameters("accountSearch", "typeEQ", Account.Type.PROJECT); - } + networksToReturn = _networksDao.search(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId, + null, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter); } - Pair, Integer> result = _networksDao.searchAndCount(mainSearchCriteria, searchFilter); - List networksToReturn = result.first(); if (supportedServicesStr != null && !supportedServicesStr.isEmpty() && !networksToReturn.isEmpty()) { - List supportedNetworks = new ArrayList<>(); - Service[] supportedServices = new Service[supportedServicesStr.size()]; + List supportedNetworks = new ArrayList(); + Service[] suppportedServices = new Service[supportedServicesStr.size()]; int i = 0; for (String supportedServiceStr : supportedServicesStr) { Service service = Service.getService(supportedServiceStr); if (service == null) { throw new InvalidParameterValueException("Invalid service specified " + supportedServiceStr); } else { - supportedServices[i] = service; + suppportedServices[i] = service; } i++; } + for (NetworkVO network : networksToReturn) { - if (areServicesSupportedInNetwork(network.getId(), supportedServices)) { + if (areServicesSupportedInNetwork(network.getId(), suppportedServices)) { supportedNetworks.add(network); } } + networksToReturn = supportedNetworks; } if (canUseForDeploy != null) { - List networksForDeploy = new ArrayList<>(); + List networksForDeploy = new ArrayList(); for (NetworkVO network : networksToReturn) { if (_networkModel.canUseForDeploy(network) == canUseForDeploy) { networksForDeploy.add(network); } } + networksToReturn = networksForDeploy; } - if (applyManualPagination) { - //Now apply pagination - List wPagination = com.cloud.utils.StringUtils.applyPagination(networksToReturn, cmd.getStartIndex(), cmd.getPageSizeVal()); - if (wPagination != null) { - Pair, Integer> listWPagination = new Pair<>(wPagination, networksToReturn.size()); - return listWPagination; - } - return new Pair<>(networksToReturn, networksToReturn.size()); + //Now apply pagination + List wPagination = com.cloud.utils.StringUtils.applyPagination(networksToReturn, cmd.getStartIndex(), cmd.getPageSizeVal()); + if (wPagination != null) { + Pair, Integer> listWPagination = new Pair, Integer>(wPagination, networksToReturn.size()); + return listWPagination; } - return new Pair<>(result.first(), result.second()); + return new Pair, Integer>(networksToReturn, networksToReturn.size()); } - private SearchCriteria createNetworkSearchCriteria(SearchBuilder sb, String keyword, Long id, - Boolean isSystem, Long zoneId, String guestIpType, String trafficType, Long physicalNetworkId, - Long networkOfferingId, String aclType, Boolean restartRequired, - Boolean specifyIpRanges, Long vpcId, Map tags, Boolean display, String vlanId, Long associatedNetworkId) { + private void addNetworksToReturnIfNotExist(final List networksToReturn, final List sharedNetworks) { + Set networkIds = networksToReturn.stream() + .map(NetworkVO::getId) + .collect(Collectors.toSet()); + List sharedNetworksToReturn = sharedNetworks.stream() + .filter(network -> ! networkIds.contains(network.getId())) + .collect(Collectors.toList()); + networksToReturn.addAll(sharedNetworksToReturn); + } + + private SearchCriteria buildNetworkSearchCriteria(SearchBuilder sb, String keyword, Long id, + Boolean isSystem, Long zoneId, String guestIpType, String trafficType, Long physicalNetworkId, + Long networkOfferingId, String aclType, boolean skipProjectNetworks, Boolean restartRequired, + Boolean specifyIpRanges, Long vpcId, Map tags, Boolean display, String vlanId, Long associatedNetworkId) { SearchCriteria sc = sb.create(); @@ -2587,6 +2569,12 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C sc.addAnd("physicalNetworkId", SearchCriteria.Op.EQ, physicalNetworkId); } + if (skipProjectNetworks) { + sc.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); + } else { + sc.setJoinParameters("accountSearch", "typeEQ", Account.Type.PROJECT); + } + if (restartRequired != null) { sc.addAnd("restartRequired", SearchCriteria.Op.EQ, restartRequired); } @@ -2627,8 +2615,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C return sc; } - private SearchCriteria getDomainLevelNetworksSearchCriteria(SearchBuilder sb, long domainId, boolean parentDomainsOnly) { - List networkIds = new ArrayList<>(); + private List listDomainLevelNetworks(SearchCriteria sc, Filter searchFilter, long domainId, boolean parentDomainsOnly) { + List networkIds = new ArrayList(); Set allowedDomains = _domainMgr.getDomainParentIds(domainId); List maps = _networkDomainDao.listDomainNetworkMapByDomain(allowedDomains.toArray()); @@ -2643,55 +2631,48 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C } if (!networkIds.isEmpty()) { - SearchCriteria domainSC = sb.create(); - domainSC.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); + SearchCriteria domainSC = _networksDao.createSearchCriteria(); domainSC.addAnd("id", SearchCriteria.Op.IN, networkIds.toArray()); domainSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Domain.toString()); - return domainSC; + + sc.addAnd("id", SearchCriteria.Op.SC, domainSC); + return _networksDao.search(sc, searchFilter); + } else { + return new ArrayList(); } - return null; } - private SearchCriteria getAccountSpecificNetworksSearchCriteria(SearchBuilder sb, - List permittedAccounts, boolean skipProjectNetworks) { - SearchCriteria accountSC = sb.create(); - if (skipProjectNetworks) { - accountSC.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); - } else { - accountSC.setJoinParameters("accountSearch", "typeEQ", Account.Type.PROJECT); - } + private List listAccountSpecificNetworks(SearchCriteria sc, Filter searchFilter, List permittedAccounts) { + SearchCriteria accountSC = _networksDao.createSearchCriteria(); if (!permittedAccounts.isEmpty()) { accountSC.addAnd("accountId", SearchCriteria.Op.IN, permittedAccounts.toArray()); } + accountSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Account.toString()); - return accountSC; + + sc.addAnd("id", SearchCriteria.Op.SC, accountSC); + return _networksDao.search(sc, searchFilter); } - private SearchCriteria getAccountSpecificNetworksByDomainPathSearchCriteria(SearchBuilder sb, - String path, boolean isRecursive, boolean skipProjectNetworks) { - SearchCriteria accountSC = sb.create(); - if (skipProjectNetworks) { - accountSC.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); - } else { - accountSC.setJoinParameters("accountSearch", "typeEQ", Account.Type.PROJECT); - } + private List listAccountSpecificNetworksByDomainPath(SearchCriteria sc, Filter searchFilter, String path, boolean isRecursive) { + SearchCriteria accountSC = _networksDao.createSearchCriteria(); accountSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Account.toString()); if (path != null) { if (isRecursive) { - accountSC.setJoinParameters("domainSearch", "path", path + "%"); + sc.setJoinParameters("domainSearch", "path", path + "%"); } else { - accountSC.setJoinParameters("domainSearch", "path", path); + sc.setJoinParameters("domainSearch", "path", path); } } - return accountSC; + sc.addAnd("id", SearchCriteria.Op.SC, accountSC); + return _networksDao.search(sc, searchFilter); } - private SearchCriteria getDomainSpecificNetworksByDomainPathSearchCriteria(SearchBuilder sb, - String path, boolean isRecursive) { + private List listDomainSpecificNetworksByDomainPath(SearchCriteria sc, Filter searchFilter, String path, boolean isRecursive) { - Set allowedDomains = new HashSet<>(); + Set allowedDomains = new HashSet(); if (path != null) { if (isRecursive) { allowedDomains = _domainMgr.getDomainChildrenIds(path); @@ -2701,7 +2682,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C } } - List networkIds = new ArrayList<>(); + List networkIds = new ArrayList(); List maps = _networkDomainDao.listDomainNetworkMapByDomain(allowedDomains.toArray()); @@ -2710,28 +2691,30 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C } if (!networkIds.isEmpty()) { - SearchCriteria domainSC = sb.create(); - domainSC.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); + SearchCriteria domainSC = _networksDao.createSearchCriteria(); domainSC.addAnd("id", SearchCriteria.Op.IN, networkIds.toArray()); domainSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Domain.toString()); - return domainSC; + + sc.addAnd("id", SearchCriteria.Op.SC, domainSC); + return _networksDao.search(sc, searchFilter); + } else { + return new ArrayList(); } - return null; } - private SearchCriteria getSharedNetworksSearchCriteria(SearchBuilder sb, List permittedAccounts) { + private List listSharedNetworks(SearchCriteria sc, Filter searchFilter, List permittedAccounts) { List sharedNetworkIds = _networkPermissionDao.listPermittedNetworkIdsByAccounts(permittedAccounts); if (!sharedNetworkIds.isEmpty()) { - SearchCriteria ssc = sb.create(); - ssc.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); + SearchCriteria ssc = _networksDao.createSearchCriteria(); ssc.addAnd("id", SearchCriteria.Op.IN, sharedNetworkIds.toArray()); - return ssc; + sc.addAnd("id", SearchCriteria.Op.SC, ssc); + return _networksDao.search(sc, searchFilter); } - return null; + return new ArrayList(); } - private SearchCriteria getSharedNetworksByDomainPathSearchCriteria(SearchBuilder sb, String path, boolean isRecursive) { - Set allowedDomains = new HashSet<>(); + private List listSharedNetworksByDomainPath(SearchCriteria sc, Filter searchFilter, String path, boolean isRecursive) { + Set allowedDomains = new HashSet(); if (path != null) { if (isRecursive) { allowedDomains = _domainMgr.getDomainChildrenIds(path); @@ -2753,13 +2736,13 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C List sharedNetworkIds = _networkPermissionDao.listPermittedNetworkIdsByAccounts(allowedAccountsList); if (!sharedNetworkIds.isEmpty()) { - SearchCriteria ssc = sb.create(); - ssc.setJoinParameters("accountSearch", "typeNEQ", Account.Type.PROJECT); + SearchCriteria ssc = _networksDao.createSearchCriteria(); ssc.addAnd("id", SearchCriteria.Op.IN, sharedNetworkIds.toArray()); - return ssc; + sc.addAnd("id", SearchCriteria.Op.SC, ssc); + return _networksDao.search(sc, searchFilter); } } - return null; + return new ArrayList(); } @Override diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 15121aa0a14..a5a1b7aae69 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2769,7 +2769,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M throw new InvalidParameterValueException("Unable to find user by id"); } final ControlledEntity account = getAccount(getUserAccountById(userId).getAccountId()); //Extracting the Account from the userID of the requested user. - checkAccess(CallContext.current().getCallingUser(), account); + User caller = CallContext.current().getCallingUser(); + preventRootDomainAdminAccessToRootAdminKeys(caller, account); + checkAccess(caller, account); Map keys = new HashMap(); keys.put("apikey", user.getApiKey()); @@ -2778,6 +2780,19 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M return keys; } + protected void preventRootDomainAdminAccessToRootAdminKeys(User caller, ControlledEntity account) { + if (isDomainAdminForRootDomain(caller) && isRootAdmin(account.getAccountId())) { + String msg = String.format("Caller Username %s does not have access to root admin keys", caller.getUsername()); + s_logger.error(msg); + throw new PermissionDeniedException(msg); + } + } + + protected boolean isDomainAdminForRootDomain(User callingUser) { + AccountVO caller = _accountDao.findById(callingUser.getAccountId()); + return caller.getType() == Account.Type.DOMAIN_ADMIN && caller.getDomainId() == Domain.ROOT_DOMAIN; + } + @Override public List listUserTwoFactorAuthenticationProviders() { return userTwoFactorAuthenticationProviders; @@ -2812,6 +2827,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } Account account = _accountDao.findById(user.getAccountId()); + preventRootDomainAdminAccessToRootAdminKeys(user, account); checkAccess(caller, null, true, account); // don't allow updating system user diff --git a/server/src/test/java/com/cloud/acl/DomainCheckerTest.java b/server/src/test/java/com/cloud/acl/DomainCheckerTest.java new file mode 100644 index 00000000000..a5ec41306d8 --- /dev/null +++ b/server/src/test/java/com/cloud/acl/DomainCheckerTest.java @@ -0,0 +1,166 @@ +// 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.acl; + +import org.apache.cloudstack.acl.ControlledEntity; +import org.apache.cloudstack.acl.SecurityChecker; +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 com.cloud.domain.dao.DomainDao; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.projects.ProjectManager; +import com.cloud.user.Account; +import com.cloud.user.AccountService; +import com.cloud.user.AccountVO; +import com.cloud.user.dao.AccountDao; +import com.cloud.utils.Ternary; + +@RunWith(MockitoJUnitRunner.class) +public class DomainCheckerTest { + + @Mock + AccountService _accountService; + @Mock + AccountDao _accountDao; + @Mock + DomainDao _domainDao; + @Mock + ProjectManager _projectMgr; + + @Spy + @InjectMocks + DomainChecker domainChecker; + + private ControlledEntity getMockedEntity(long accountId) { + ControlledEntity entity = Mockito.mock(Account.class); + Mockito.when(entity.getAccountId()).thenReturn(accountId); + Mockito.when(entity.getEntityType()).thenReturn((Class)Account.class); + return entity; + } + + @Test + public void testRootAdminHasAccess() { + Account rootAdmin = Mockito.mock(Account.class); + Mockito.when(rootAdmin.getId()).thenReturn(1L); + ControlledEntity entity = getMockedEntity(2L); + Mockito.when(_accountService.isRootAdmin(rootAdmin.getId())).thenReturn(true); + + domainChecker.validateCallerHasAccessToEntityOwner(rootAdmin, entity, SecurityChecker.AccessType.ModifyProject); + } + + @Test + public void testCallerIsOwner() { + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getId()).thenReturn(1L); + ControlledEntity entity = getMockedEntity(1L); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ModifyProject); + } + + @Test(expected = PermissionDeniedException.class) + public void testOwnerNotFound() { + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getId()).thenReturn(1L); + ControlledEntity entity = getMockedEntity(2L); + Mockito.when(_accountDao.findById(entity.getAccountId())).thenReturn(null); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ModifyProject); + } + + @Test + public void testDomainAdminHasAccess() { + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getId()).thenReturn(1L); + Mockito.when(caller.getDomainId()).thenReturn(100L); + Mockito.when(caller.getType()).thenReturn(Account.Type.DOMAIN_ADMIN); + ControlledEntity entity = getMockedEntity(2L); + AccountVO owner = Mockito.mock(AccountVO.class); + Mockito.when(owner.getDomainId()).thenReturn(101L); + Mockito.when(_accountDao.findById(entity.getAccountId())).thenReturn(owner); + Mockito.when(_domainDao.isChildDomain(100L, 101L)).thenReturn(true); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ModifyProject); + } + + private Ternary getProjectAccessCheckResources() { + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getId()).thenReturn(100L); + Mockito.when(caller.getType()).thenReturn(Account.Type.PROJECT); + ControlledEntity entity = getMockedEntity(2L); + AccountVO projectAccount = Mockito.mock(AccountVO.class); + Mockito.when(projectAccount.getId()).thenReturn(2L); + Mockito.when(projectAccount.getType()).thenReturn(Account.Type.PROJECT); + return new Ternary<>(caller, entity, projectAccount); + } + + @Test + public void testProjectOwnerCanModify() { + Ternary resources = getProjectAccessCheckResources(); + Account caller = resources.first(); + ControlledEntity entity = resources.second(); + AccountVO projectAccount = resources.third(); + Mockito.when(_accountDao.findById(entity.getAccountId())).thenReturn(projectAccount); + Mockito.when(_projectMgr.canModifyProjectAccount(caller, projectAccount.getId())).thenReturn(true); + Mockito.doReturn(true).when(domainChecker).checkOperationPermitted(caller, entity); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ModifyProject); + } + + @Test(expected = PermissionDeniedException.class) + public void testProjectOwnerCannotModify() { + Ternary resources = getProjectAccessCheckResources(); + Account caller = resources.first(); + ControlledEntity entity = resources.second(); + AccountVO projectAccount = resources.third(); + Mockito.when(_accountDao.findById(entity.getAccountId())).thenReturn(projectAccount); + Mockito.when(_projectMgr.canModifyProjectAccount(caller, projectAccount.getId())).thenReturn(false); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ModifyProject); + } + + @Test + public void testProjectOwnerCanAccess() { + Ternary resources = getProjectAccessCheckResources(); + Account caller = resources.first(); + ControlledEntity entity = resources.second(); + AccountVO projectAccount = resources.third(); + Mockito.when(_accountDao.findById(entity.getAccountId())).thenReturn(projectAccount); + Mockito.when(_projectMgr.canAccessProjectAccount(caller, projectAccount.getId())).thenReturn(true); + Mockito.doReturn(true).when(domainChecker).checkOperationPermitted(caller, entity); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ListEntry); + } + + @Test(expected = PermissionDeniedException.class) + public void testProjectOwnerCannotAccess() { + Ternary resources = getProjectAccessCheckResources(); + Account caller = resources.first(); + ControlledEntity entity = resources.second(); + AccountVO projectAccount = resources.third(); + Mockito.when(_accountDao.findById(entity.getAccountId())).thenReturn(projectAccount); + Mockito.when(_projectMgr.canAccessProjectAccount(caller, projectAccount.getId())).thenReturn(false); + + domainChecker.validateCallerHasAccessToEntityOwner(caller, entity, SecurityChecker.AccessType.ListEntry); + } + +} diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index e5c623ca6df..9d780096abf 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -28,6 +28,8 @@ import java.util.Map; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd; + +import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import org.apache.cloudstack.api.response.UserTwoFactorAuthenticationSetupResponse; @@ -291,6 +293,63 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { accountManagerImpl.getKeys(_listkeyscmd); } + @Test(expected = PermissionDeniedException.class) + public void testGetUserKeysCmdDomainAdminRootAdminUser() { + CallContext.register(callingUser, callingAccount); + Mockito.when(_listkeyscmd.getID()).thenReturn(2L); + Mockito.when(accountManagerImpl.getActiveUser(2L)).thenReturn(userVoMock); + Mockito.when(userAccountDaoMock.findById(2L)).thenReturn(userAccountVO); + Mockito.when(userAccountVO.getAccountId()).thenReturn(2L); + Mockito.when(userDetailsDaoMock.listDetailsKeyPairs(Mockito.anyLong())).thenReturn(null); + + // Queried account - admin account + AccountVO adminAccountMock = Mockito.mock(AccountVO.class); + Mockito.when(adminAccountMock.getAccountId()).thenReturn(2L); + Mockito.when(_accountDao.findByIdIncludingRemoved(2L)).thenReturn(adminAccountMock); + Mockito.lenient().when(accountService.isRootAdmin(2L)).thenReturn(true); + Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), + Mockito.nullable(ControlledEntity.class), Mockito.nullable(AccessType.class), Mockito.anyString())).thenReturn(true); + + // Calling account is domain admin of the ROOT domain + Mockito.lenient().when(callingAccount.getType()).thenReturn(Account.Type.DOMAIN_ADMIN); + Mockito.lenient().when(callingAccount.getDomainId()).thenReturn(Domain.ROOT_DOMAIN); + + Mockito.lenient().when(callingUser.getAccountId()).thenReturn(2L); + Mockito.lenient().when(_accountDao.findById(2L)).thenReturn(callingAccount); + + Mockito.lenient().when(accountService.isDomainAdmin(Mockito.anyLong())).thenReturn(Boolean.TRUE); + Mockito.lenient().when(accountMock.getAccountId()).thenReturn(2L); + + accountManagerImpl.getKeys(_listkeyscmd); + } + + @Test + public void testPreventRootDomainAdminAccessToRootAdminKeysNormalUser() { + User user = Mockito.mock(User.class); + ControlledEntity entity = Mockito.mock(ControlledEntity.class); + Mockito.when(user.getAccountId()).thenReturn(1L); + AccountVO account = Mockito.mock(AccountVO.class); + Mockito.when(account.getType()).thenReturn(Account.Type.NORMAL); + Mockito.when(_accountDao.findById(1L)).thenReturn(account); + accountManagerImpl.preventRootDomainAdminAccessToRootAdminKeys(user, entity); + Mockito.verify(accountManagerImpl, Mockito.never()).isRootAdmin(Mockito.anyLong()); + } + + @Test(expected = PermissionDeniedException.class) + public void testPreventRootDomainAdminAccessToRootAdminKeysRootDomainAdminUser() { + User user = Mockito.mock(User.class); + ControlledEntity entity = Mockito.mock(ControlledEntity.class); + Mockito.when(user.getAccountId()).thenReturn(1L); + AccountVO account = Mockito.mock(AccountVO.class); + Mockito.when(account.getType()).thenReturn(Account.Type.DOMAIN_ADMIN); + Mockito.when(account.getDomainId()).thenReturn(Domain.ROOT_DOMAIN); + Mockito.when(_accountDao.findById(1L)).thenReturn(account); + Mockito.when(entity.getAccountId()).thenReturn(1L); + Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), + Mockito.nullable(ControlledEntity.class), Mockito.nullable(AccessType.class), Mockito.anyString())).thenReturn(true); + accountManagerImpl.preventRootDomainAdminAccessToRootAdminKeys(user, entity); + } + @Test public void updateUserTestTimeZoneAndEmailNull() { prepareMockAndExecuteUpdateUserTest(0); diff --git a/test/integration/smoke/test_account_access.py b/test/integration/smoke/test_account_access.py new file mode 100644 index 00000000000..97eeced6386 --- /dev/null +++ b/test/integration/smoke/test_account_access.py @@ -0,0 +1,198 @@ +# 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. +""" BVT tests for Account User Access +""" +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import * +from marvin.lib.base import (Account, + User, + Domain) +from marvin.lib.common import (get_domain) +from marvin.cloudstackAPI import (getUserKeys) +from marvin.cloudstackException import CloudstackAPIException +from nose.plugins.attrib import attr + +_multiprocess_shared_ = True + +class TestAccountAccess(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testClient = super(TestAccountAccess, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls.services = testClient.getParsedTestDataConfig() + cls.hypervisor = testClient.getHypervisorInfo() + cls._cleanup = [] + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + + cls.domains = [] + cls.domain_admins = {} + cls.domain_users = {} + cls.account_users = {} + + domain_data = { + "name": "domain_1" + } + cls.domain_1 = Domain.create( + cls.apiclient, + domain_data, + ) + cls._cleanup.append(cls.domain_1) + cls.domains.append(cls.domain_1) + domain_data["name"] = "domain_11" + cls.domain_11 = Domain.create( + cls.apiclient, + domain_data, + parentdomainid=cls.domain_1.id + ) + cls._cleanup.append(cls.domain_11) + cls.domains.append(cls.domain_11) + domain_data["name"] = "domain_12" + cls.domain_12 = Domain.create( + cls.apiclient, + domain_data, + parentdomainid=cls.domain_1.id + ) + cls._cleanup.append(cls.domain_12) + cls.domains.append(cls.domain_12) + domain_data["name"] = "domain_2" + cls.domain_2 = Domain.create( + cls.apiclient, + domain_data, + ) + cls._cleanup.append(cls.domain_2) + cls.domains.append(cls.domain_2) + + + for d in cls.domains: + cls.create_domainadmin_and_user(d) + + @classmethod + def tearDownClass(cls): + super(TestAccountAccess, cls).tearDownClass() + + @classmethod + def create_account(cls, domain, is_admin): + cls.debug(f"Creating account for domain {domain.name}, admin: {is_admin}") + data = { + "email": "admin-" + domain.name + "@test.com", + "firstname": "Admin", + "lastname": domain.name, + "username": "admin-" + domain.name, + "password": "password" + } + if is_admin == False: + data["email"] = "user-" + domain.name + "@test.com" + data["firstname"] = "User" + data["username"] = "user-" + domain.name + account = Account.create( + cls.apiclient, + data, + admin=is_admin, + domainid=domain.id + ) + cls._cleanup.append(account) + if is_admin == True: + cls.domain_admins[domain.id] = account + else: + cls.domain_users[domain.id] = account + + user = User.create( + cls.apiclient, + data, + account=account.name, + domainid=account.domainid) + cls._cleanup.append(user) + cls.account_users[account.id] = user + + @classmethod + def create_domainadmin_and_user(cls, domain): + cls.debug(f"Creating accounts for domain #{domain.id} {domain.name}") + cls.create_account(domain, True) + cls.create_account(domain, False) + + def get_user_keys(self, api_client, user_id): + getUserKeysCmd = getUserKeys.getUserKeysCmd() + getUserKeysCmd.id = user_id + return api_client.getUserKeys(getUserKeysCmd) + + def is_child_domain(self, parent_domain, child_domain): + if not parent_domain or not child_domain: + return False + parent_domain_prefix = parent_domain.split('-')[0] + child_domain_prefix = child_domain.split('-')[0] + if not parent_domain_prefix or not child_domain_prefix: + return False + return child_domain_prefix.startswith(parent_domain_prefix) + + + @attr(tags=["advanced", "advancedns", "smoke", "sg"], required_hardware="false") + def test_01_user_access(self): + """ + Test user account is not accessing any other account + """ + + domain_user_accounts = [value for value in self.domain_users.values()] + all_account_users = [value for value in self.account_users.values()] + for user_account in domain_user_accounts: + current_account_user = self.account_users[user_account.id] + self.debug(f"Check for account {user_account.name} with user {current_account_user.username}") + user_api_client = self.testClient.getUserApiClient( + UserName=user_account.name, + DomainName=user_account.domain + ) + for user in all_account_users: + self.debug(f"Checking access for user {user.username} associated with account {user.account}") + try: + self.get_user_keys(user_api_client, user.id) + self.debug(f"API successful") + if user.id != current_account_user.id: + self.fail(f"User account #{user_account.id} was able to access another account #{user.id}") + except CloudstackAPIException as e: + self.debug(f"Exception occurred: {e}") + if user.id == current_account_user.id: + self.fail(f"User account #{user_account.id} not able to access own account") + + @attr(tags=["advanced", "advancedns", "smoke", "sg"], required_hardware="false") + def test_02_domain_admin_access(self): + """ + Test domain admin account is not accessing any other account from unauthorized domain + """ + + domain_admin_accounts = [value for value in self.domain_admins.values()] + all_account_users = [value for value in self.account_users.values()] + for admin_account in domain_admin_accounts: + current_account_user = self.account_users[admin_account.id] + self.debug(f"Check for domain admin {admin_account.name} with user {current_account_user.username}, {current_account_user.domain}") + admin_api_client = self.testClient.getUserApiClient( + UserName=admin_account.name, + DomainName=admin_account.domain + ) + for user in all_account_users: + self.debug(f"Checking access for user {user.username}, {user.domain} associated with account {user.account}") + try: + self.get_user_keys(admin_api_client, user.id) + self.debug(f"API successful") + if self.is_child_domain(current_account_user.domain, user.domain) == False: + self.fail(f"User account #{admin_account.id} was able to access another account #{user.id}") + except CloudstackAPIException as e: + self.debug(f"Exception occurred: {e}") + if self.is_child_domain(current_account_user.domain, user.domain) == True: + self.fail(f"User account #{admin_account.id} not able to access own account") diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 8f2c9fa6d15..d67cd9819f7 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -3058,6 +3058,7 @@ "message.failed.to.remove": "Failed to remove", "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.", "message.guest.traffic.in.advanced.zone": "Guest Network traffic is communication between end-user Instances. Specify a range of VLAN IDs or VXLAN Network identifiers (VNIs) to carry guest traffic for each physical Network.", "message.guest.traffic.in.basic.zone": "Guest Network traffic is communication between end-user Instances. Specify a range of IP addresses that CloudStack can assign to guest Instances. Make sure this range does not overlap the reserved system IP range.", "message.host.controlstate": "The Control Plane Status of this Instance is ", diff --git a/ui/src/components/view/ListView.vue b/ui/src/components/view/ListView.vue index 2a379b5bf52..68e4bc0ffca 100644 --- a/ui/src/components/view/ListView.vue +++ b/ui/src/components/view/ListView.vue @@ -93,9 +93,6 @@ - @@ -174,7 +171,10 @@ {{ text }} {{ text }} -