Livnat Peer has posted comments on this change. Change subject: core: Add console reconnect permission ......................................................................
Patch Set 7: (2 inline comments) This change looks very close, I agree with Omer's comments and added a small one of my own :) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java Line 50: permissions.add(new PermissionSubject(MultiLevelAdministrationHandler.SYSTEM_OBJECT_ID, VdcObjectType.System, ActionGroup.RECONNECT_TO_VM)); Adding to the above: We have object hierarchy in the permissions mechanism. When giving a permissions on an object like SYSTEM it means it propagates to all VMs in the set-up, If you give permission on cluster it propagates to all VMs in that cluster etc. In the above usage we 'look' for permission (VS. 'giving' permissions), we state on which object we are looking to have the permission. If we look for permission on VM, we'll be ok if the user gave the permissions on the VM, it's cluster, it's DC or system. When looking for permission we should state the lowest Object in the hierarchy we are ok with, in the case it's VM: permissions.add(new PermissionSubject( vmId, VdcObjectType.VM, ActionGroup.RECONNECT_TO_VM)); Line 60: */ I would rename this method to reflect better it's current purpose, I would use: needPermissionForConnectingToConsole - returns true if we need to add permission check false otherwise -- To view, visit http://gerrit.ovirt.org/995 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I027b5ff23589b712917c78b7b799b7a3f965e61a Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
