Federico Simoncelli has posted comments on this change. Change subject: backend: [wip] add ActionGroup to access image domains ......................................................................
Patch Set 1: (5 comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java Line 134: @Override Line 135: public List<PermissionSubject> getPermissionCheckSubjects() { Line 136: List<PermissionSubject> permissionSubjects = new ArrayList<>(); Line 137: permissionSubjects.add(new PermissionSubject(getDiskImage().getId(), Line 138: VdcObjectType.Disk, ActionGroup.ATTACH_DISK)); No, this is the source (regular data domains with disks). Line 139: permissionSubjects.add(new PermissionSubject(getParameters().getStorageDomainId(), Line 140: VdcObjectType.Storage, ActionGroup.CREATE_DISK)); // ActionGroup.ACCESS_IMAGE_STORAGE ? Line 141: return permissionSubjects; Line 142: } Line 136: List<PermissionSubject> permissionSubjects = new ArrayList<>(); Line 137: permissionSubjects.add(new PermissionSubject(getDiskImage().getId(), Line 138: VdcObjectType.Disk, ActionGroup.ATTACH_DISK)); Line 139: permissionSubjects.add(new PermissionSubject(getParameters().getStorageDomainId(), Line 140: VdcObjectType.Storage, ActionGroup.CREATE_DISK)); // ActionGroup.ACCESS_IMAGE_STORAGE ? It depends if we want to distinguish between readers (ACCESS_IMAGE_STORAGE) and writers (users that can *create* new images). I think we should keep CREATE_DISK here, or split ACCESS_IMAGE_STORAGE in READ_IMAGE_STORAGE and WRITE_IMAGE_STORAGE. Maybe we could generalize them (at least in the name): CREATE_DISK => CREATE_DOMAIN_ITEMS ACCESS_IMAGE_STORAGE => ACCESS_DOMAIN_ITEMS Line 141: return permissionSubjects; Line 142: } Line 143: Line 144: @Override .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java Line 96: MANIPULATE_GLUSTER_HOOK(1003, RoleType.ADMIN, VdcObjectType.GlusterHook, true, ApplicationMode.GlusterOnly), Line 97: MANIPULATE_GLUSTER_SERVICE(1004, RoleType.ADMIN, VdcObjectType.GlusterService, true, ApplicationMode.GlusterOnly), Line 98: Line 99: // Disks action groups Line 100: CREATE_DISK(1100, RoleType.USER, VdcObjectType.Storage, false, ApplicationMode.VirtOnly), CREATE_DISK is the ability to create a disk, you can't put the ability to create a disk on the disk itself (which is obviously already existing). Line 101: ATTACH_DISK(1101, RoleType.USER, VdcObjectType.Disk, true, ApplicationMode.VirtOnly), Line 102: EDIT_DISK_PROPERTIES(1102, RoleType.USER, VdcObjectType.Disk, true, ApplicationMode.VirtOnly), Line 103: CONFIGURE_DISK_STORAGE(1103, RoleType.USER, VdcObjectType.Disk, true, ApplicationMode.VirtOnly), Line 104: DELETE_DISK(1104, RoleType.USER, VdcObjectType.Disk, true, ApplicationMode.VirtOnly), .................................................... File packaging/dbscripts/upgrade/03_03_0780_image_domains_permissions.sql Line 1: -- Adding the ACCESS_IMAGE_STORAGE action to the relevant roles Line 2: INSERT INTO roles_groups (role_id, action_group_id) VALUES Line 3: ('00000000-0000-0000-0001-000000000001', 1106), -- UserRole I think so, what I do (at least in my testing) is giving the UserRole on a specific image domain to an user. That user will be able to use (access) the specific image storage domain. Line 4: ('def00008-0000-0000-0000-def000000008', 1106), -- TemplateAdmin Line 5: ('00000000-0000-0000-0000-000000000001', 1106), -- SuperUser Line 6: ('def00003-0000-0000-0000-def000000003', 1106), -- StorageAdmin Line 3: ('00000000-0000-0000-0001-000000000001', 1106), -- UserRole Line 4: ('def00008-0000-0000-0000-def000000008', 1106), -- TemplateAdmin Line 5: ('00000000-0000-0000-0000-000000000001', 1106), -- SuperUser Line 6: ('def00003-0000-0000-0000-def000000003', 1106), -- StorageAdmin Line 7: ('00000000-0000-0000-0001-000000000002', 1106); -- PowerUserRole Same as above. Not sure if we need two, but for sure if we give the access permission to the UserRole, we have to give it to the PowerUserRole too. -- To view, visit http://gerrit.ovirt.org/18078 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifbff053962ae1dceef51c7d8ff356fcf527aa5e2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches