[pve-devel] [PATCH-SERIES manager] improve bulk action permissions

public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed

* [pve-devel] [PATCH-SERIES manager] improve bulk action permissions
@ 2023-03-01 14:22 Fiona Ebner
  2023-03-01 14:22 ` [pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend Fiona Ebner
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Fiona Ebner @ 2023-03-01 14:22 UTC (permalink / raw)
  To: pve-devel

In the UI, fix the checks to use the same permission as the backend,
i.e. VM.PowerMgmt rather than Sys.PowerMgmt.

In the backend, also allow the bulk action when the user has the
appropriate permission for each guest in the passed-in list.


Fiona Ebner (2):
  ui: bulk start/stop: align capability checks with backend
  api: node: bulk actions: allow when user has permission for each guest

 PVE/API2/Nodes.pm            | 39 +++++++++++++++++++++++++++++++++---
 www/manager6/node/CmdMenu.js |  4 +++-
 www/manager6/node/Config.js  |  6 +++---
 3 files changed, 42 insertions(+), 7 deletions(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend
  2023-03-01 14:22 [pve-devel] [PATCH-SERIES manager] improve bulk action permissions Fiona Ebner
@ 2023-03-01 14:22 ` Fiona Ebner
  2023-03-01 14:22 ` [pve-devel] [PATCH manager 2/2] api: node: bulk actions: allow when user has permission for each guest Fiona Ebner
  2023-03-15 17:22 ` [pve-devel] applied-series: [PATCH-SERIES manager] improve bulk action permissions Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Fiona Ebner @ 2023-03-01 14:22 UTC (permalink / raw)
  To: pve-devel

The backend requires VM.PowerMgmt, not Sys.PowerMgmt for bulk start
and bulk stop.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/node/CmdMenu.js | 4 +++-
 www/manager6/node/Config.js  | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/www/manager6/node/CmdMenu.js b/www/manager6/node/CmdMenu.js
index 71548e9c..dc56ef08 100644
--- a/www/manager6/node/CmdMenu.js
+++ b/www/manager6/node/CmdMenu.js
@@ -126,9 +126,11 @@ Ext.define('PVE.node.CmdMenu', {
 	if (!caps.vms['VM.Migrate']) {
 	    me.getComponent('bulkmigrate').setDisabled(true);
 	}
-	if (!caps.nodes['Sys.PowerMgmt']) {
+	if (!caps.vms['VM.PowerMgmt']) {
 	    me.getComponent('bulkstart').setDisabled(true);
 	    me.getComponent('bulkstop').setDisabled(true);
+	}
+	if (!caps.nodes['Sys.PowerMgmt']) {
 	    me.getComponent('wakeonlan').setDisabled(true);
 	}
 	if (!caps.nodes['Sys.Console']) {
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 9269e892..0cc23fb4 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -34,13 +34,13 @@ Ext.define('PVE.node.Config', {
 	var actionBtn = Ext.create('Ext.Button', {
 	    text: gettext('Bulk Actions'),
 	    iconCls: 'fa fa-fw fa-ellipsis-v',
-	    disabled: !caps.nodes['Sys.PowerMgmt'] && !caps.vms['VM.Migrate'],
+	    disabled: !caps.vms['VM.PowerMgmt'] && !caps.vms['VM.Migrate'],
 	    menu: new Ext.menu.Menu({
 		items: [
 		    {
 			text: gettext('Bulk Start'),
 			iconCls: 'fa fa-fw fa-play',
-			disabled: !caps.nodes['Sys.PowerMgmt'],
+			disabled: !caps.vms['VM.PowerMgmt'],
 			handler: function() {
 			    Ext.create('PVE.window.BulkAction', {
 				autoShow: true,
@@ -54,7 +54,7 @@ Ext.define('PVE.node.Config', {
 		    {
 			text: gettext('Bulk Shutdown'),
 			iconCls: 'fa fa-fw fa-stop',
-			disabled: !caps.nodes['Sys.PowerMgmt'],
+			disabled: !caps.vms['VM.PowerMgmt'],
 			handler: function() {
 			    Ext.create('PVE.window.BulkAction', {
 				autoShow: true,
-- 
2.30.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH manager 2/2] api: node: bulk actions: allow when user has permission for each guest
  2023-03-01 14:22 [pve-devel] [PATCH-SERIES manager] improve bulk action permissions Fiona Ebner
  2023-03-01 14:22 ` [pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend Fiona Ebner
@ 2023-03-01 14:22 ` Fiona Ebner
  2023-03-15 17:22 ` [pve-devel] applied-series: [PATCH-SERIES manager] improve bulk action permissions Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Fiona Ebner @ 2023-03-01 14:22 UTC (permalink / raw)
  To: pve-devel

Users with permissions for some guests can already start a task for
each sequentially.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Nodes.pm | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 47c2d741..c9bf2831 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1756,7 +1756,9 @@ __PACKAGE__->register_method ({
     method => 'POST',
     protected => 1,
     permissions => {
-	check => ['perm', '/', [ 'VM.PowerMgmt' ]],
+	description => "The 'VM.PowerMgmt' permission is required on '/' or on '/vms/<ID>' for "
+	    ."each ID passed via the 'vms' parameter.",
+	user => 'all',
     },
     proxyto => 'node',
     description => "Start all VMs and containers located on this node (by default only those with onboot=1).",
@@ -1786,6 +1788,15 @@ __PACKAGE__->register_method ({
 	my $rpcenv = PVE::RPCEnvironment::get();
 	my $authuser = $rpcenv->get_user();
 
+	if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt' ], 1)) {
+	    my @vms = PVE::Tools::split_list($param->{vms});
+	    if (scalar(@vms) > 0) {
+		$rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt' ]) for @vms;
+	    } else {
+		raise_perm_exc("/, VM.PowerMgmt");
+	    }
+	}
+
 	my $nodename = $param->{node};
 	$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
 
@@ -1891,7 +1902,9 @@ __PACKAGE__->register_method ({
     method => 'POST',
     protected => 1,
     permissions => {
-	check => ['perm', '/', [ 'VM.PowerMgmt' ]],
+	description => "The 'VM.PowerMgmt' permission is required on '/' or on '/vms/<ID>' for "
+	    ."each ID passed via the 'vms' parameter.",
+	user => 'all',
     },
     proxyto => 'node',
     description => "Stop all VMs and Containers.",
@@ -1930,6 +1943,15 @@ __PACKAGE__->register_method ({
 	my $rpcenv = PVE::RPCEnvironment::get();
 	my $authuser = $rpcenv->get_user();
 
+	if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt' ], 1)) {
+	    my @vms = PVE::Tools::split_list($param->{vms});
+	    if (scalar(@vms) > 0) {
+		$rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt' ]) for @vms;
+	    } else {
+		raise_perm_exc("/, VM.PowerMgmt");
+	    }
+	}
+
 	my $nodename = $param->{node};
 	$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
 
@@ -2056,7 +2078,9 @@ __PACKAGE__->register_method ({
     proxyto => 'node',
     protected => 1,
     permissions => {
-	check => ['perm', '/', [ 'VM.Migrate' ]],
+	description => "The 'VM.Migrate' permission is required on '/' or on '/vms/<ID>' for each "
+	    ."ID passed via the 'vms' parameter.",
+	user => 'all',
     },
     description => "Migrate all VMs and Containers.",
     parameters => {
@@ -2092,6 +2116,15 @@ __PACKAGE__->register_method ({
 	my $rpcenv = PVE::RPCEnvironment::get();
 	my $authuser = $rpcenv->get_user();
 
+	if (!$rpcenv->check($authuser, "/", [ 'VM.Migrate' ], 1)) {
+	    my @vms = PVE::Tools::split_list($param->{vms});
+	    if (scalar(@vms) > 0) {
+		$rpcenv->check($authuser, "/vms/$_", [ 'VM.Migrate' ]) for @vms;
+	    } else {
+		raise_perm_exc("/, VM.Migrate");
+	    }
+	}
+
 	my $nodename = $param->{node};
 	$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied-series: [PATCH-SERIES manager] improve bulk action permissions
  2023-03-01 14:22 [pve-devel] [PATCH-SERIES manager] improve bulk action permissions Fiona Ebner
  2023-03-01 14:22 ` [pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend Fiona Ebner
  2023-03-01 14:22 ` [pve-devel] [PATCH manager 2/2] api: node: bulk actions: allow when user has permission for each guest Fiona Ebner
@ 2023-03-15 17:22 ` Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2023-03-15 17:22 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner

Am 01/03/2023 um 15:22 schrieb Fiona Ebner:
> In the UI, fix the checks to use the same permission as the backend,
> i.e. VM.PowerMgmt rather than Sys.PowerMgmt.
> 
> In the backend, also allow the bulk action when the user has the
> appropriate permission for each guest in the passed-in list.
> 
> 
> Fiona Ebner (2):
>   ui: bulk start/stop: align capability checks with backend
>   api: node: bulk actions: allow when user has permission for each guest
> 
>  PVE/API2/Nodes.pm            | 39 +++++++++++++++++++++++++++++++++---
>  www/manager6/node/CmdMenu.js |  4 +++-
>  www/manager6/node/Config.js  |  6 +++---
>  3 files changed, 42 insertions(+), 7 deletions(-)
> 


applied series, thanks!




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-03-15 17:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-01 14:22 [pve-devel] [PATCH-SERIES manager] improve bulk action permissions Fiona Ebner
2023-03-01 14:22 ` [pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend Fiona Ebner
2023-03-01 14:22 ` [pve-devel] [PATCH manager 2/2] api: node: bulk actions: allow when user has permission for each guest Fiona Ebner
2023-03-15 17:22 ` [pve-devel] applied-series: [PATCH-SERIES manager] improve bulk action permissions Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Service provided by Proxmox Server Solutions GmbH | Privacy | Legal