[pve-devel] [PATCH manager] ui: cloudinit: disallow unprivileged users to regenerate images

[pve-devel] [PATCH manager] ui: cloudinit: disallow unprivileged users to regenerate images

[Daniel Kral]

Disables the "Regenerate image" button in the VM CloudInit tab for
users, which lack the necessary permissions to do so, which are
"VM.Config.CloudInit" and "VM.Config.CDROM". This is checked by the VM
config update API endpoint in qemu-server, when ejecting and re-adding
CloudInit drive images.

This is consistent with the permissions restricting access to adding
CloudInit drives in the Hardware tab.

This is a cosmetic change as the VM config update API endpoint would
fail because of insufficient permissions anyway.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
I stumbled upon this while checking on BugZilla #3105 and trying to use
the "Regenerate image" button with a user, that had only VM.Audit and
VM.Config.CloudInit permissions. where the user missed the
"VM.Config.CDROM" permission and the action couldn't be completed.

 www/manager6/qemu/CloudInit.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/www/manager6/qemu/CloudInit.js b/www/manager6/qemu/CloudInit.js
index 49519726..aeef8c15 100644
--- a/www/manager6/qemu/CloudInit.js
+++ b/www/manager6/qemu/CloudInit.js
@@ -142,7 +142,10 @@ Ext.define('PVE.qemu.CloudInit', {
 		}
 	});
 
-	me.down('#savebtn').setDisabled(!found);
+	let caps = Ext.state.Manager.get('GuiCap');
+	let canSaveImage = !!caps.vms['VM.Config.CDROM'] && !!caps.vms['VM.Config.Cloudinit'];
+	me.down('#savebtn').setDisabled(!found || !canSaveImage);
+
 	me.setDisabled(!found);
 	if (!found) {
 	    me.getView().mask(gettext('No CloudInit Drive found'), ['pve-static-mask']);
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager] ui: cloudinit: disallow unprivileged users to regenerate images

[Fiona Ebner]

Am 31.10.24 um 15:06 schrieb Daniel Kral:
> Disables the "Regenerate image" button in the VM CloudInit tab for
> users, which lack the necessary permissions to do so, which are
> "VM.Config.CloudInit" and "VM.Config.CDROM". This is checked by the VM
> config update API endpoint in qemu-server, when ejecting and re-adding
> CloudInit drive images.
> 
> This is consistent with the permissions restricting access to adding
> CloudInit drives in the Hardware tab.
> 
> This is a cosmetic change as the VM config update API endpoint would
> fail because of insufficient permissions anyway.
> 

Since those users can already use the cloudinit_update endpoint (just
not via UI) and thus effectively don't need "VM.Config.CDROM", I think
it's better to change the UI to use that endpoint as well.

The endpoint was added here [0] and the switch in the UI was already
proposed [1] as part of that but it seems never applied.

[0]:
https://lore.proxmox.com/pve-devel/20220622115206.3295425-1-aderumier@odiso.com/

[1]:
https://lore.proxmox.com/pve-devel/20220622141335.3616129-2-aderumier@odiso.com/

> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> I stumbled upon this while checking on BugZilla #3105 and trying to use
> the "Regenerate image" button with a user, that had only VM.Audit and
> VM.Config.CloudInit permissions. where the user missed the
> "VM.Config.CDROM" permission and the action couldn't be completed.
> 
>  www/manager6/qemu/CloudInit.js | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/qemu/CloudInit.js b/www/manager6/qemu/CloudInit.js
> index 49519726..aeef8c15 100644
> --- a/www/manager6/qemu/CloudInit.js
> +++ b/www/manager6/qemu/CloudInit.js
> @@ -142,7 +142,10 @@ Ext.define('PVE.qemu.CloudInit', {
>  		}
>  	});
>  
> -	me.down('#savebtn').setDisabled(!found);
> +	let caps = Ext.state.Manager.get('GuiCap');
> +	let canSaveImage = !!caps.vms['VM.Config.CDROM'] && !!caps.vms['VM.Config.Cloudinit'];
> +	me.down('#savebtn').setDisabled(!found || !canSaveImage);
> +
>  	me.setDisabled(!found);
>  	if (!found) {
>  	    me.getView().mask(gettext('No CloudInit Drive found'), ['pve-static-mask']);


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager] ui: cloudinit: disallow unprivileged users to regenerate images

[Daniel Kral]

On 11/4/24 13:02, Fiona Ebner wrote:
> Since those users can already use the cloudinit_update endpoint (just
> not via UI) and thus effectively don't need "VM.Config.CDROM", I think
> it's better to change the UI to use that endpoint as well.
> 
> The endpoint was added here [0] and the switch in the UI was already
> proposed [1] as part of that but it seems never applied.
> 
> [0]:
> https://lore.proxmox.com/pve-devel/20220622115206.3295425-1-aderumier@odiso.com/
> 
> [1]:
> https://lore.proxmox.com/pve-devel/20220622141335.3616129-2-aderumier@odiso.com/

Thanks for pointing that out! I also think that it would be a better 
solution. I've already applied the UI patches locally, but since the 
cloudinit-related API endpoint's have been changed significantly since 
then, I will update those parts of the patch and send it anew as a v2 
with an added Originally-by trailer.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel