Tomas Jelinek has uploaded a new change for review. Change subject: engine: copy vm/template permissions fixes ......................................................................
engine: copy vm/template permissions fixes Changes: - set the copyVmPermissions on all required places on FE - the new permission is not assigned to the user creating the vm/template but to the user it has been assigned before - made sure that the same permission is not added twice (to not violate the unique constraint on the permissions table Change-Id: Ibe9d1b305c1275f695b954d52b8f49f4f775f119 Signed-off-by: Tomas Jelinek <[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/AddVmTemplateCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UniquePermissionsSet.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/permissions.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java 6 files changed, 111 insertions(+), 38 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/69/17869/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 6de9374..b91db2a 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 @@ -7,7 +7,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; - import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.job.ExecutionHandler; @@ -46,9 +45,9 @@ import org.ovirt.engine.core.common.businessentities.VmStatistics; import org.ovirt.engine.core.common.businessentities.VmType; import org.ovirt.engine.core.common.businessentities.VmWatchdog; -import org.ovirt.engine.core.common.businessentities.permissions; import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNic; +import org.ovirt.engine.core.common.businessentities.permissions; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBLLException; @@ -817,24 +816,28 @@ } protected void addVmPermission() { + UniquePermissionsSet permissionsToAdd = new UniquePermissionsSet(); if ((getParameters()).isMakeCreatorExplicitOwner()) { - permissions perms = new permissions(getCurrentUser().getUserId(), PredefinedRoles.VM_OPERATOR.getId(), + permissionsToAdd.addPermission(getCurrentUser().getUserId(), PredefinedRoles.VM_OPERATOR.getId(), getVmId(), VdcObjectType.VM); - MultiLevelAdministrationHandler.addPermission(perms); - getCompensationContext().snapshotNewEntity(perms); } if (getParameters().isCopyTemplatePermissions() && !getVmTemplateId().equals(VmTemplateHandler.BlankVmTemplateId)) { - copyTemplatePermissions(); + copyTemplatePermissions(permissionsToAdd); + } + + if (!permissionsToAdd.isEmpty()) { + List<permissions> permissionsList = permissionsToAdd.asPermissionList(); + MultiLevelAdministrationHandler.addPermission(permissionsList.toArray(new permissions[permissionsList.size()])); + + getCompensationContext().snapshotNewEntities(permissionsList); } } - private void copyTemplatePermissions() { + private void copyTemplatePermissions(UniquePermissionsSet permissionsToAdd) { PermissionDAO dao = getDbFacade().getPermissionDao(); List<permissions> templatePermissions = dao.getAllForEntity(getVmTemplateId(), getCurrentUser().getUserId(), false); - - List<permissions> vmPermissions = new ArrayList<permissions>(); for (permissions templatePermission : templatePermissions) { boolean templateOwnerRole = templatePermission.getrole_id().equals(PredefinedRoles.TEMPLATE_OWNER.getId()); @@ -844,15 +847,10 @@ continue; } - vmPermissions.add(new permissions(getCurrentUser().getUserId(), templatePermission.getrole_id(), - getVmId(), VdcObjectType.VM)); - + permissionsToAdd.addPermission(templatePermission.getad_element_id(), templatePermission.getrole_id(), + getVmId(), VdcObjectType.VM); } - if (vmPermissions.size() > 0) { - MultiLevelAdministrationHandler.addPermission(vmPermissions.toArray(new permissions[vmPermissions.size()])); - getCompensationContext().snapshotNewEntities(vmPermissions); - } } protected void addDiskPermissions(List<DiskImage> newDiskImages) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java index ca8eb42..4f46eb2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java @@ -8,7 +8,6 @@ import java.util.List; import java.util.Map; import java.util.Set; - import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaSanityParameter; @@ -43,21 +42,21 @@ import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.businessentities.VmTemplateStatus; import org.ovirt.engine.core.common.businessentities.VmType; -import org.ovirt.engine.core.common.businessentities.permissions; import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNic; +import org.ovirt.engine.core.common.businessentities.permissions; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.validation.group.CreateEntity; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; import org.ovirt.engine.core.dao.PermissionDAO; import org.ovirt.engine.core.utils.collections.MultiValueMapUtils; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; -import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; -import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; @DisableInPrepareMode @NonTransactiveCommandAttribute(forceCompensation = true) @@ -574,16 +573,23 @@ } private void addPermission() { - addPermissionForTemplate(getCurrentUser().getUserId(), PredefinedRoles.TEMPLATE_OWNER); + UniquePermissionsSet permissionsToAdd = new UniquePermissionsSet(); + + addPermissionForTemplate(permissionsToAdd, getCurrentUser().getUserId(), PredefinedRoles.TEMPLATE_OWNER); // if the template is for public use, set EVERYONE as a TEMPLATE_USER. if (getParameters().isPublicUse()) { - addPermissionForTemplate(MultiLevelAdministrationHandler.EVERYONE_OBJECT_ID, PredefinedRoles.TEMPLATE_USER); + addPermissionForTemplate(permissionsToAdd, MultiLevelAdministrationHandler.EVERYONE_OBJECT_ID, PredefinedRoles.TEMPLATE_USER); } - copyVmPermissions(); + copyVmPermissions(permissionsToAdd); + + if (!permissionsToAdd.isEmpty()) { + List<permissions> permissionsList = permissionsToAdd.asPermissionList(); + MultiLevelAdministrationHandler.addPermission(permissionsList.toArray(new permissions[permissionsList.size()])); + } } - private void copyVmPermissions() { + private void copyVmPermissions(UniquePermissionsSet permissionsToAdd) { if (!isVmInDb || !getParameters().isCopyVmPermissions()) { return; } @@ -592,25 +598,15 @@ List<permissions> vmPermissions = dao.getAllForEntity(getVmId(), getCurrentUser().getUserId(), false); - List<permissions> templatePermissions = new ArrayList<permissions>(); - for (permissions vmPermission : vmPermissions) { - templatePermissions.add(new permissions(getCurrentUser().getUserId(), vmPermission.getrole_id(), - getParameters().getVmTemplateId(), VdcObjectType.VmTemplate)); + permissionsToAdd.addPermission(vmPermission.getad_element_id(), vmPermission.getrole_id(), + getParameters().getVmTemplateId(), VdcObjectType.VmTemplate); } - if (templatePermissions.size() > 0) { - MultiLevelAdministrationHandler.addPermission(templatePermissions.toArray(new permissions[templatePermissions.size()])); - } } - private void addPermissionForTemplate(Guid userId, PredefinedRoles role) { - permissions perms = new permissions(); - perms.setad_element_id(userId); - perms.setObjectType(VdcObjectType.VmTemplate); - perms.setObjectId(getParameters().getVmTemplateId()); - perms.setrole_id(role.getId()); - MultiLevelAdministrationHandler.addPermission(perms); + private void addPermissionForTemplate(UniquePermissionsSet permissionsToAdd, Guid userId, PredefinedRoles role) { + permissionsToAdd.addPermission(userId, role.getId(), getParameters().getVmTemplateId(), VdcObjectType.VmTemplate); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UniquePermissionsSet.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UniquePermissionsSet.java new file mode 100644 index 0000000..bf8f3c3 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UniquePermissionsSet.java @@ -0,0 +1,75 @@ +package org.ovirt.engine.core.bll; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import org.ovirt.engine.core.common.VdcObjectType; +import org.ovirt.engine.core.common.businessentities.permissions; +import org.ovirt.engine.core.common.utils.ObjectUtils; +import org.ovirt.engine.core.compat.Guid; + +/** + * A set which ensures that the unique key of permissions table will not be violated + */ +public class UniquePermissionsSet extends HashSet<PermissionWithUniqueKeyEquals> { + + public void addPermission(Guid adElementId, Guid roleId, Guid objectId, VdcObjectType objectType) { + add(new PermissionWithUniqueKeyEquals(adElementId, roleId, objectId, objectType)); + } + + /** + * Converts to instances of permissions class - the rest of the code expects the getClass() to be permissions + * and not it's child + */ + public List<permissions> asPermissionList() { + List<permissions> res = new ArrayList<permissions>(); + + for (PermissionWithUniqueKeyEquals permission : this) { + res.add(permission.asPermission()); + } + + return res; + } + +} + +/** + * Permission which offers the same equals as the unique key constraint on the permissions table + */ +class PermissionWithUniqueKeyEquals extends permissions { + + public PermissionWithUniqueKeyEquals(Guid adElementId, Guid roleId, Guid objectId, VdcObjectType objectType) { + super(adElementId, roleId, objectId, objectType); + } + + public permissions asPermission() { + return new permissions(getad_element_id(), getrole_id(), getObjectId(), getObjectType()); + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((getad_element_id() == null) ? 0 : getad_element_id().hashCode()); + result = prime * result + ((getObjectId() == null) ? 0 : getObjectId().hashCode()); + result = prime * result + ((getrole_id() == null) ? 0 : getrole_id().hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + permissions other = (permissions) obj; + return (ObjectUtils.objectsEqual(getad_element_id(), other.getad_element_id()) + && ObjectUtils.objectsEqual(getrole_id(), other.getrole_id()) + && ObjectUtils.objectsEqual(getObjectId(), other.getObjectId())); + } +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/permissions.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/permissions.java index 449b853..67e8ac2 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/permissions.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/permissions.java @@ -167,4 +167,6 @@ && ObjectUtils.objectsEqual(roleId, other.roleId)); } + + } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java index bf4323b..8f12e80 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java @@ -1128,6 +1128,7 @@ VmManagementParametersBase param = new VmManagementParametersBase(gettempVm()); param.setDiskInfoDestinationMap(model.getDisksAllocationModel().getImageToDestinationDomainMap()); param.setMakeCreatorExplicitOwner(true); + param.setCopyTemplatePermissions((Boolean) model.getCopyPermissions().getEntity()); param.setSoundDeviceEnabled((Boolean) model.getIsSoundcardEnabled().getEntity()); param.setConsoleEnabled((Boolean) model.getIsConsoleDeviceEnabled().getEntity()); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java index b6c48fa..c9f19b8 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java @@ -2090,6 +2090,7 @@ params.setDiskInfoDestinationMap(model.getDisksAllocationModel().getImageToDestinationDomainMap()); params.setConsoleEnabled((Boolean) model.getIsConsoleDeviceEnabled().getEntity()); params.setBalloonEnabled(balloonEnabled(model)); + params.setCopyTemplatePermissions((Boolean) model.getCopyPermissions().getEntity()); ArrayList<VdcActionParametersBase> parameters = new ArrayList<VdcActionParametersBase>(); parameters.add(params); -- To view, visit http://gerrit.ovirt.org/17869 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibe9d1b305c1275f695b954d52b8f49f4f775f119 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
