[PATCH storage v2 1/2] api: prune backups: align permission check for dry run and active run

[Fiona Ebner <f.ebner@proxmox.com>]

The API endpoint for listing storage content filters volumes with
check_volume_access(). The dry run for prune backups should not return
volumes that the content listing does not show. While
check_volume_access() could be used there as well, access to a backup
volume already requires 'Datastore.AllocateSpace' for the storage and
'VM.Backup' for the guest, or 'Datastore.Allocate' for the storage.
These are the same permissions required for the actual pruning.

The dry run should show what an actual pruning invocation with the
same parameters would do. Using check_volume_access() would, for
invocations without an explicit guest ID, show what a storage-wide
prune would do for all guest groups the user has access to. This would
be in contrast to the actual pruning invocation, which requires
'Datastore.Allocate' for an invocation without an explicit guest ID,
so there would be a mismatch.

Together, these points suggest aligning the permission check between
the endpoint for the dry run and the endpoint for the active run.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Storage/PruneBackups.pm | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/PVE/API2/Storage/PruneBackups.pm b/src/PVE/API2/Storage/PruneBackups.pm
index 2920c899..7863c36e 100644
--- a/src/PVE/API2/Storage/PruneBackups.pm
+++ b/src/PVE/API2/Storage/PruneBackups.pm
@@ -12,6 +12,19 @@ use PVE::Tools qw(extract_param);
 
 use base qw(PVE::RESTHandler);
 
+my sub assert_prune_permission {
+    my ($rpcenv, $authuser, $storeid, $vmid) = @_;
+
+    if (defined($vmid)) {
+        $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
+        $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']);
+    } else {
+        $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']);
+    }
+
+    return;
+}
+
 __PACKAGE__->register_method({
     name => 'dryrun',
     path => '',
@@ -20,12 +33,9 @@ __PACKAGE__->register_method({
         "Get prune information for backups. NOTE: this is only a preview and might not be "
         . "what a subsequent prune call does if backups are removed/added in the meantime.",
     permissions => {
-        check => [
-            'perm',
-            '/storage/{storage}',
-            ['Datastore.Audit', 'Datastore.AllocateSpace'],
-            any => 1,
-        ],
+        description => "You need the 'Datastore.Allocate' privilege on the storage "
+            . "(or if a VM ID is specified, 'Datastore.AllocateSpace' and 'VM.Backup' for the VM).",
+        user => 'all',
     },
     protected => 1,
     proxyto => 'node',
@@ -100,6 +110,9 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
+        my $rpcenv = PVE::RPCEnvironment::get();
+        my $authuser = $rpcenv->get_user();
+
         my $cfg = PVE::Storage::config();
 
         my $vmid = extract_param($param, 'vmid');
@@ -110,6 +123,8 @@ __PACKAGE__->register_method({
         $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
             if defined($prune_backups);
 
+        assert_prune_permission($rpcenv, $authuser, $storeid, $vmid);
+
         return PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, $vmid, $type, 1);
     },
 });
@@ -177,12 +192,7 @@ __PACKAGE__->register_method({
         $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
             if defined($prune_backups);
 
-        if (defined($vmid)) {
-            $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
-            $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']);
-        } else {
-            $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']);
-        }
+        assert_prune_permission($rpcenv, $authuser, $storeid, $vmid);
 
         my $id = (defined($vmid) ? "$vmid@" : '') . $storeid;
         my $worker = sub {
-- 
2.47.3