RodrigoDLopez commented on a change in pull request #4584:
URL: https://github.com/apache/cloudstack/pull/4584#discussion_r558354265
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java
##########
@@ -52,30 +52,33 @@
@Parameter(name=ApiConstants.ID, type=CommandType.UUID,
entityType=VolumeResponse.class, description="the ID of the disk volume")
private Long id;
- @Parameter(name = ApiConstants.PATH, type = CommandType.STRING,
description = "The path of the volume")
+ @Parameter(name = ApiConstants.PATH, type = CommandType.STRING,
description = "The path of the volume", authorized = {RoleType.Admin})
private String path;
@Parameter(name = ApiConstants.CHAIN_INFO,
type = CommandType.STRING,
description = "The chain info of the volume",
- since = "4.4")
+ since = "4.4", authorized = {RoleType.Admin})
Review comment:
Correct me if I'm wrong, but I believe that only administrators can
execute the updateVolume command.
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/volume/UpdateVolumeCmd.java
##########
@@ -52,30 +52,33 @@
@Parameter(name=ApiConstants.ID, type=CommandType.UUID,
entityType=VolumeResponse.class, description="the ID of the disk volume")
private Long id;
- @Parameter(name = ApiConstants.PATH, type = CommandType.STRING,
description = "The path of the volume")
+ @Parameter(name = ApiConstants.PATH, type = CommandType.STRING,
description = "The path of the volume", authorized = {RoleType.Admin})
private String path;
@Parameter(name = ApiConstants.CHAIN_INFO,
type = CommandType.STRING,
description = "The chain info of the volume",
- since = "4.4")
+ since = "4.4", authorized = {RoleType.Admin})
private String chainInfo;
@Parameter(name = ApiConstants.STORAGE_ID,
type = CommandType.UUID,
entityType = StoragePoolResponse.class,
description = "Destination storage pool UUID for the volume",
- since = "4.3")
+ since = "4.3", authorized = {RoleType.Admin})
private Long storageId;
- @Parameter(name = ApiConstants.STATE, type = CommandType.STRING,
description = "The state of the volume", since = "4.3")
+ @Parameter(name = ApiConstants.STATE, type = CommandType.STRING,
description = "The state of the volume", since = "4.3", authorized =
{RoleType.Admin})
private String state;
@Parameter(name = ApiConstants.DISPLAY_VOLUME,
type = CommandType.BOOLEAN,
description = "an optional field, whether to the display the volume to the
end user or not.", authorized = {RoleType.Admin})
private Boolean displayVolume;
+ @Parameter(name = ApiConstants.NAME, type = CommandType.STRING,
description = "new name of the volume", since = "4.16")
Review comment:
You put roleAdmin in all the other parameters of this API, but not this
one.
Any reason for this behavior? users without administrator permission will
not be able to run this command. And ordinary users shouldn't be able to change
the name of what doesn't belong to them.
It would be nice to capitalize the n in new as well
##########
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##########
@@ -1763,7 +1763,15 @@ public Volume attachVolumeToVM(Long vmId, Long volumeId,
Long deviceId) {
@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, String customId, long entityOwnerId, String
chainInfo) {
+ public Volume updateVolume(long volumeId, String path, String state, Long
storageId, Boolean displayVolume,
+ 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");
Review comment:
does it make sense to allow ordinary users the power to change names
within the environment?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
