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

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

[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(-)

-- 
2.30.2

[pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend

[Fiona Ebner]

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

[pve-devel] [PATCH manager 2/2] api: node: bulk actions: allow when user has permission for each guest

[Fiona Ebner]

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

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

[Thomas Lamprecht]

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!