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

Reply via email to