ofri masad has uploaded a new change for review.

Change subject: core: Fix unauthorized move-disk
......................................................................

core: Fix unauthorized move-disk

When quota was set to 'Enforcing' user could move a disk to a storage
domain which he had no permissions to use.

For some reason, an 'if' statement was used in order to add the
storage-domain to the permission subject list only when quota was set to
'Disabled' or 'Audit'. 'If' statement was canceled. The storage-domain
is added to the list when disk is about to be moved to the
storage-domain.

Change-Id: I32713cbcf3206ab449ddca9b0ebd382bf77a7582
Signed-off-by: Ofri Masad <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
4 files changed, 7 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/69/14769/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
index 777f494..42226bb 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
@@ -722,9 +722,8 @@
         if (getVmTemplate() != null && 
!getVmTemplate().getDiskList().isEmpty()) {
             for (DiskImage disk : 
getParameters().getDiskInfoDestinationMap().values()) {
                 if (disk.getStorageIds() != null && 
!disk.getStorageIds().isEmpty()) {
-                    addStoragePermissionByQuotaMode(permissionList,
-                            GuidUtils.getGuidValue(getStoragePoolId()),
-                            
GuidUtils.getGuidValue(disk.getStorageIds().get(0)));
+                    permissionList.add(new 
PermissionSubject(GuidUtils.getGuidValue(getStoragePoolId()),
+                            VdcObjectType.Storage, ActionGroup.CREATE_DISK));
                 }
             }
         }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
index 1840efa..224bcca 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
@@ -54,7 +54,6 @@
 import 
org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot.EntityStatusSnapshot;
 import 
org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot.SnapshotType;
 import org.ovirt.engine.core.common.businessentities.QuotaEnforcementTypeEnum;
-import org.ovirt.engine.core.common.businessentities.StoragePool;
 import org.ovirt.engine.core.common.businessentities.tags;
 import org.ovirt.engine.core.common.errors.VdcBLLException;
 import org.ovirt.engine.core.common.errors.VdcBllErrors;
@@ -1754,33 +1753,6 @@
 
         if (commandLock == null) {
             commandLock = context.getLock();
-        }
-    }
-
-    /**
-     * Adds Storage domain to permissions subjects unless Quota is enabled.
-     * Method is used for storage consumption related commands.
-     *
-     * @param permsList
-     *            the permissions list
-     * @param storagePoolId
-     *            the storage pool id
-     * @param StorageDomainId
-     *            the storage domain
-     */
-    protected void addStoragePermissionByQuotaMode(List<PermissionSubject> 
permsList,
-            Guid storagePoolId,
-            Guid StorageDomainId) {
-
-        StoragePool storagePool = null;
-        if (storagePoolId != null) {
-            storagePool = getStoragePoolDAO().get(storagePoolId);
-            if (storagePool != null) {
-                if (storagePool.getQuotaEnforcementType() == 
QuotaEnforcementTypeEnum.DISABLED
-                        || storagePool.getQuotaEnforcementType() == 
QuotaEnforcementTypeEnum.SOFT_ENFORCEMENT) {
-                    permsList.add(new PermissionSubject(StorageDomainId, 
VdcObjectType.Storage, ActionGroup.CREATE_DISK));
-                }
-            }
         }
     }
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
index 27cde95..45945c0 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
@@ -298,10 +298,8 @@
             DiskImage image = getImage();
             Guid diskId = image == null ? Guid.Empty : image.getId();
             permsList.add(new PermissionSubject(diskId, VdcObjectType.Disk, 
ActionGroup.CONFIGURE_DISK_STORAGE));
-
-            addStoragePermissionByQuotaMode(permsList,
-                    getStoragePoolId().getValue(),
-                    getParameters().getStorageDomainId());
+            permsList.add(new 
PermissionSubject(getParameters().getStorageDomainId(),
+                    VdcObjectType.Storage, ActionGroup.CREATE_DISK));
         }
         return permsList;
     }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
index c43266f..a02067d 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
@@ -91,12 +91,9 @@
             permissionList.add(new PermissionSubject(parameters.getImageId(),
                     VdcObjectType.Disk,
                     ActionGroup.CONFIGURE_DISK_STORAGE));
-
-            setStoragePoolId(getVm().getStoragePoolId());
-
-            addStoragePermissionByQuotaMode(permissionList,
-                    getStoragePoolId().getValue(),
-                    parameters.getStorageDomainId());
+            permissionList.add(new 
PermissionSubject(parameters.getStorageDomainId(),
+                    VdcObjectType.Storage,
+                    ActionGroup.CREATE_DISK));
         }
 
         return permissionList;


--
To view, visit http://gerrit.ovirt.org/14769
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I32713cbcf3206ab449ddca9b0ebd382bf77a7582
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to