Omer Frenkel has posted comments on this change. Change subject: core: Add console reconnect permission ......................................................................
Patch Set 7: (2 inline comments) .................................................... 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)); sorry for repeating this, but im not sure we got to an agreement on this: requesting the permission on the VM rather than on the system will give the users more granularity with regards to who can reconnect, if requesting it on the system, it means that the user must have it on the system, and then can reconnect to any vm. requesting on the vm, will allow the admin still giving it to someone on the system in order to reconnect to any vm, but in addition, the ability to give a user the permission to reconnect only to a specific vm Line 65: if (vm.getAllowConsoleReconnect()) { since now this code happens before canDoAction, it has to be null safe, i would add a check that the vm is not null, and if it is, just return false, the user will get an authorization error. -- 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