From 365f0f720cbcb53cb70318a9c1b15e10f291728a Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Sat, 18 Apr 2020 08:23:04 +0200
Subject: [PATCH] fix permission tests using non-uri parameters

We nood to do those tests inside the fuction body instead...
---
 src/api2/backup.rs | 14 ++++++++++++--
 src/api2/pull.rs   | 12 ++++++++----
 src/api2/reader.rs | 13 +++++++++++--
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/src/api2/backup.rs b/src/api2/backup.rs
index 38af92af..76d08438 100644
--- a/src/api2/backup.rs
+++ b/src/api2/backup.rs
@@ -15,6 +15,7 @@ use crate::server::{WorkerTask, H2Service};
 use crate::backup::*;
 use crate::api2::types::*;
 use crate::config::acl::PRIV_DATASTORE_ALLOCATE_SPACE;
+use crate::config::cached_user_info::CachedUserInfo;
 
 mod environment;
 use environment::*;
@@ -38,7 +39,11 @@ pub const API_METHOD_UPGRADE_BACKUP: ApiMethod = ApiMethod::new(
             ("debug", true, &BooleanSchema::new("Enable verbose debug logging.").schema()),
         ]),
     )
-).access(None, &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_ALLOCATE_SPACE, false));
+).access(
+    // Note: parameter 'store' is no uri parameter, so we need to test inside function body
+    Some("The user needs Datastore.AllocateSpace privilege on /datastore/{store}."),
+    &Permission::Anybody
+);
 
 fn upgrade_to_backup_protocol(
     parts: Parts,
@@ -51,7 +56,13 @@ fn upgrade_to_backup_protocol(
     async move {
     let debug = param["debug"].as_bool().unwrap_or(false);
 
+    let username = rpcenv.get_user().unwrap();
+
     let store = tools::required_string_param(&param, "store")?.to_owned();
+
+    let user_info = CachedUserInfo::new()?;
+    user_info.check_privs(&username, &["datastore", &store], PRIV_DATASTORE_ALLOCATE_SPACE, false)?;
+
     let datastore = DataStore::lookup_datastore(&store)?;
 
     let backup_type = tools::required_string_param(&param, "backup-type")?;
@@ -74,7 +85,6 @@ fn upgrade_to_backup_protocol(
 
     let worker_id = format!("{}_{}_{}", store, backup_type, backup_id);
 
-    let username = rpcenv.get_user().unwrap();
     let env_type = rpcenv.env_type();
 
     let backup_group = BackupGroup::new(backup_type, backup_id);
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index 8b867367..b8d47901 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -17,6 +17,7 @@ use crate::client::*;
 use crate::config::remote;
 use crate::api2::types::*;
 use crate::config::acl::PRIV_DATASTORE_ALLOCATE_SPACE;
+use crate::config::cached_user_info::CachedUserInfo;
 
 // fixme: implement filters
 // fixme: delete vanished groups
@@ -389,10 +390,9 @@ pub async fn pull_store(
         },
     },
     access: {
-        permission: &Permission::And(&[
-            &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_ALLOCATE_SPACE, false),
-            &Permission::Privilege(&["remote", "{store}"], PRIV_DATASTORE_ALLOCATE_SPACE, false),
-        ]),
+        // Note: used parameters are no uri parameters, so we need to test inside function body
+        description: "The user needs Datastore.AllocateSpace privilege on '/datastore/{store}' and '/remote/{remote}/{remote-store}'.",
+        permission: &Permission::Anybody,
     },
 )]
 /// Sync store from other repository
@@ -405,7 +405,11 @@ async fn pull (
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<String, Error> {
 
+    let user_info = CachedUserInfo::new()?;
+
     let username = rpcenv.get_user().unwrap();
+    user_info.check_privs(&username, &["datastore", &store], PRIV_DATASTORE_ALLOCATE_SPACE, false)?;
+    user_info.check_privs(&username, &["remote", &remote, &remote_store], PRIV_DATASTORE_ALLOCATE_SPACE, false)?;
 
     let delete = delete.unwrap_or(true);
 
diff --git a/src/api2/reader.rs b/src/api2/reader.rs
index 085e700f..72e32cc5 100644
--- a/src/api2/reader.rs
+++ b/src/api2/reader.rs
@@ -16,6 +16,7 @@ use crate::backup::*;
 use crate::server::{WorkerTask, H2Service};
 use crate::tools;
 use crate::config::acl::PRIV_DATASTORE_ALLOCATE_SPACE;
+use crate::config::cached_user_info::CachedUserInfo;
 
 mod environment;
 use environment::*;
@@ -42,7 +43,11 @@ pub const API_METHOD_UPGRADE_BACKUP: ApiMethod = ApiMethod::new(
             ("debug", true, &BooleanSchema::new("Enable verbose debug logging.").schema()),
         ]),
     )
-).access(None, &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_ALLOCATE_SPACE, false));
+).access(
+    // Note: parameter 'store' is no uri parameter, so we need to test inside function body
+    Some("The user needs Datastore.AllocateSpace privilege on /datastore/{store}."),
+    &Permission::Anybody
+);
 
 fn upgrade_to_backup_reader_protocol(
     parts: Parts,
@@ -55,7 +60,12 @@ fn upgrade_to_backup_reader_protocol(
     async move {
         let debug = param["debug"].as_bool().unwrap_or(false);
 
+        let username = rpcenv.get_user().unwrap();
         let store = tools::required_string_param(&param, "store")?.to_owned();
+
+        let user_info = CachedUserInfo::new()?;
+        user_info.check_privs(&username, &["datastore", &store], PRIV_DATASTORE_ALLOCATE_SPACE, false)?;
+
         let datastore = DataStore::lookup_datastore(&store)?;
 
         let backup_type = tools::required_string_param(&param, "backup-type")?;
@@ -76,7 +86,6 @@ fn upgrade_to_backup_reader_protocol(
             bail!("unexpected http version '{:?}' (expected version < 2)", parts.version);
         }
 
-        let username = rpcenv.get_user().unwrap();
         let env_type = rpcenv.env_type();
 
         let backup_dir = BackupDir::new(backup_type, backup_id, backup_time);