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

Reply via email to