Am 05/04/2024 um 15:13 schrieb Friedrich Weber: > On 04/04/2024 17:20, Thomas Lamprecht wrote: >> Or does it even make sense to check this at all? >> As long as the user has the rights to execute a stop they probably >> should also be able to force it at any time, even for other users? > > The usecase sounds reasonable, but would technically amount to a small > privilege escalation: Currently, if user A and user B both have > PVEVMUser and user A starts a `vzshutdown` task, user B must have > `Sys.Modify` to kill the task. >
Hmm, yeah, this is definitively something one could argue about as we currently do not have much overruling functionality of tasks triggered by other users that can manage a specific resource, but not the whole system. So this is essential new territory w.r.t. semantics, and we can also adapt new ones, albeit they should certainly not be completely unexpected. >> Maybe make this optional and only enforce it for users that do not >> have some more powerful priv? > > We could introduce a similar check as for the DELETE > /nodes/{node}/tasks/{upid} endpoint: Users can always overrule their own > guest tasks, and with `Sys.Modify` they can overrule any guest task (for > guests for which they have the necessary permission). Yeah, I think that would be a safe bet for now. > Still, right now I think the primary motivation for this overruling > feature is to save GUI users some frustration and/or clicks. In this > scenario, a user will overrule only their own tasks, which is possible > with the current check. What do you think about keeping the check as it > is for now, and making it more permissive once the need arises? I think that allowing users that hold the respective Sys.Modify and VM.PowerMgmt to overrule any tasks from the start wouldn't be to much "speculative future-proofing" but rather something expected while still safe. FWIW, you could also drop the $authuser then and just get it from the RPCEnv singleton available in all API call-paths and then do the permission check in the helper directly. This would IMO be also a bit better w.r.t. conveying why we do it this way. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel