[pdm-devel] [PATCH datacenter-manager] api: remote tasks: add fine-grained permission check on the remote level

* [pdm-devel] [PATCH datacenter-manager] api: remote tasks: add fine-grained permission check on the remote level
@ 2025-11-28 15:15 Lukas Wagner
  2025-11-28 18:53 ` [pdm-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Lukas Wagner @ 2025-11-28 15:15 UTC (permalink / raw)
  To: pdm-devel

Adds a check for the remote task APIs that requires a user to have
Resource.Audit on /resource/{remote}.

Could make sense to a.) verify permissions on the node-level (we do so
in PVE) and b.) allow a user to see their own tasks. But we can do that
later as well, more important for now is getting rid of the bare
Permission::Anybody.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/api/remote_tasks.rs | 32 ++++++++++++++++++++++++++------
 server/src/remote_tasks/mod.rs |  5 +++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/server/src/api/remote_tasks.rs b/server/src/api/remote_tasks.rs
index 02b6383b..1dff4bd6 100644
--- a/server/src/api/remote_tasks.rs
+++ b/server/src/api/remote_tasks.rs
@@ -27,10 +27,9 @@ const SUBDIRS: SubdirMap = &sorted!([
 ]);
 
 #[api(
-    // FIXME:: see list-like API calls in resource routers, we probably want more fine-grained
-    // checks..
     access: {
         permission: &Permission::Anybody,
+        description: "Resource.Audit privileges on /resource/{remote} are needed to list tasks from a given remote."
     },
     input: {
         properties: {
@@ -64,16 +63,26 @@ async fn list_tasks(
         user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
     }
 
-    let tasks = remote_tasks::get_tasks(filters, remote, view).await?;
+    let check_privs = move |remote_name: &str| {
+        user_info
+            .check_privs(
+                &auth_id,
+                &["resource", remote_name],
+                PRIV_RESOURCE_AUDIT,
+                false,
+            )
+            .is_ok()
+    };
+
+    let tasks = remote_tasks::get_tasks(filters, remote, check_privs, view).await?;
 
     Ok(tasks)
 }
 
 #[api(
-    // FIXME:: see list-like API calls in resource routers, we probably want more fine-grained
-    // checks..
     access: {
         permission: &Permission::Anybody,
+        description: "Resource.Audit privileges on /resource/{remote} are needed to list tasks from a given remote."
     },
     input: {
         properties: {
@@ -106,7 +115,18 @@ async fn task_statistics(
         user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
     }
 
-    let tasks = remote_tasks::get_tasks(filters, remote, view).await?;
+    let check_privs = move |remote_name: &str| {
+        user_info
+            .check_privs(
+                &auth_id,
+                &["resource", remote_name],
+                PRIV_RESOURCE_AUDIT,
+                false,
+            )
+            .is_ok()
+    };
+
+    let tasks = remote_tasks::get_tasks(filters, remote, check_privs, view).await?;
 
     let mut by_type: HashMap<String, TaskCount> = HashMap::new();
     let mut by_remote: HashMap<String, TaskCount> = HashMap::new();
diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index ea3c5539..b080811f 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -31,6 +31,7 @@ const NUMBER_OF_UNCOMPRESSED_FILES: u32 = 2;
 pub async fn get_tasks(
     filters: TaskFilters,
     remote_filter: Option<String>,
+    check_privs: impl Fn(&str) -> bool + Send + 'static,
     view: Option<String>,
 ) -> Result<Vec<TaskListItem>, Error> {
     let view = views::get_optional_view(view.as_deref())?;
@@ -64,6 +65,8 @@ pub async fn get_tasks(
                             if !view.is_node_included(task.upid.remote(), &pve_upid.node) {
                                 return None;
                             }
+                        } else if !check_privs(task.upid.remote()) {
+                            return None;
                         }
                         Some(TaskListItem {
                             upid: task.upid.to_string(),
@@ -83,6 +86,8 @@ pub async fn get_tasks(
                             if !view.is_node_included(task.upid.remote(), &pbs_upid.node) {
                                 return None;
                             }
+                        } else if !check_privs(task.upid.remote()) {
+                            return None;
                         }
                         Some(TaskListItem {
                             upid: task.upid.to_string(),
-- 
2.47.3



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


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