all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [RFC PATCH datacenter-manager 0/3] ui: acl: pre-populate permission path selector
@ 2026-03-25 15:35 Shan Shaji
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 1/3] pdm-client: add `list_views` function to fetch views list Shan Shaji
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Shan Shaji @ 2026-03-25 15:35 UTC (permalink / raw)
  To: pdm-devel

vikunja ticket: #858

Creating this series as an RFC because, after populating the resource paths
and testing them, AFAIU some of the permissions were not working because of
our current API level permissions.

I have tested the cases listed below. Would like to confirm if the desired and
expected behaviour that i mentioned is correct and if I have missed anything. Since i
was not sure about the correct behaviour I haven't made any updates in the API
level permissions.

Backup Server
=============
remote_name = backup
node_name = localhost
Role = Auditor
datastore_name = backup

- /resource/backup
	- This was working even without enabling the propagate flag. Not sure if
	  that is expected.
	- Desired behaviour: when propagate flag is enabled, the resources need to
	  be shown, else the remote should be visible under the remotes but the
	  resources shouldn't be visible.  
- /resource/backup/datastore/backup
	- The datastore details are not shown either in the remote dashboard or in
	  the main dashboard.
	- Desired behaviour: 
		- The remote needs to be visible under remotes of the sidebar.
		- The datastore details need to be visible under the remote overview section.
		- Datastore status needs to be visible under the main dashboard.    
- /resource/backup/node 
	- propagate enabled: not working
	- propagate disabled: not working
	- Desired behaviour:
		- The remote should be visible under the remotes in the sidebar.
		- When propagate is enabled, all the resources in the specific node needs to
		  be visible.
- /resource/backup/node/localhost
	- not working even when propagate is enabled.
	- Desired behaviour: similar behaviour to /resource/backup/node

Cluster
=======
remote_name = cluster
nodes = 3
Role = Auditor
guests = 5

- /resource/cluster
	- The remote was shown in the dashboard. However, I was getting a 403
	  error (user has no access to resource list). 
	- When the propagate flag is set, the resource list loads correctly.
- /resource/cluster/guest
	- propagate disabled: Not working (expected)
	- Not working even when the propagate flag is set to true. The remote is not
	  listed under the remotes in the sidebar. Also, the guest statuses were not
	  being shown in the dashboard as well.
	- Desired behaviour: when propagate is enabled, all virtual guests inside the
		specific remote need to be visible.
- /resource/cluster/guest/{100,101,102}
	- not working, the guest status was not shown in the main dashboard or in the
	  remote-specific dashboard.
	- The propagate option doesn't matter here as it's a specific resource.
	- Desired behaviour: The guest status needs to be shown in the dashboard.
		The remote needs to be listed under remotes.
- /resource/cluster/node
	- propagate disabled: Not working (expected)
	- propagate enabled: Not working
	- Desired behaviour: When propagate is enabled, the remote needs to be visible
		under remotes and the node resources need to be visible
		inside the remote dashboard.
- /resource/cluster/node/pve-node-1
	- propagate disabled: Not working (expected)
	- propagate enabled: Not working
	- Desired behaviour: when propagate is enabled, the remote needs to be
		visible under remotes and the node-specific resources
		need to be visible under the remote dashboard.

PVE node
========
remote_name = trial
Role = Auditor
guests = 1
node_name = pve-free-trial

- /resource/trial
	- propagate disabled: 403 error (user has no access to resource list). (expected)
	- propagate enabled: The resources are visible under the dashboard. (expected)
- /resource/trial/guest
	- propagate disabled: No remote under remotes and no status under the
	  main dashboard. (expected)
	- propagate enabled: No remote under remotes and no status under the main
	 	dashboard.
	- Desired behaviour: when propagate is enabled, the guest statuses need to be
		shown.
- /resource/trial/guest/100
	- propagate doesn't matter here as it's a specific resource.
	- not working, the guest status is not being shown inside the remote-specific
	  dashboard or in the main dashboard.
- /resource/trial/node
	- propagate disabled: no resources are loaded. (expected)
	- propagate enabled: no resources are loaded. (not expected)
	- Desired behaviour: The node resources need to be visible under the remote-
		specific dashboard and in the main dashboard when propagate is enabled. 
- /resource/trial/node/pve-free-trial
	- propagate disabled: no resources are loaded. (expected)
	- propagate enabled: no resources are loaded. (not expected)
	- Desired behaviour: The node resources need to be visible under the remote
		dashboard and in the main dashboard when propagate is enabled. 

Views
=====
views_count = 2

- /view/layout
	- User can view the specific view. 
- /view
	- propagate enabled: Both views are visible to the user. 
	- propagate disabled: Views are visible under the views in the sidebar but the
		resources are not visible when clicking on each view.

Shan Shaji (3):
  pdm-client: add `list_views` function to fetch views list
  ui: acl: list granular level permission paths for views
  ui: acl: list granular level permission paths for resources

 lib/pdm-api-types/src/acl.rs                  |   3 +-
 lib/pdm-client/src/lib.rs                     |  10 ++
 .../configuration/permission_path_selector.rs | 125 +++++++++++++++---
 3 files changed, 121 insertions(+), 17 deletions(-)

-- 
2.47.3





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

* [RFC PATCH datacenter-manager 1/3] pdm-client: add `list_views` function to fetch views list
  2026-03-25 15:35 [RFC PATCH datacenter-manager 0/3] ui: acl: pre-populate permission path selector Shan Shaji
@ 2026-03-25 15:35 ` Shan Shaji
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 2/3] ui: acl: list granular level permission paths for views Shan Shaji
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources Shan Shaji
  2 siblings, 0 replies; 8+ messages in thread
From: Shan Shaji @ 2026-03-25 15:35 UTC (permalink / raw)
  To: pdm-devel

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 lib/pdm-client/src/lib.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 0fee97a..1565869 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -1366,6 +1366,16 @@ impl<T: HttpApiClient> PdmClient<T> {
             .expect_json()?
             .data)
     }
+
+    /// Get the list of views.
+    pub async fn list_views(&self) -> Result<Vec<pdm_api_types::views::ViewConfig>, Error> {
+        Ok(self
+            .0
+            .get("/api2/extjs/config/views")
+            .await?
+            .expect_json()?
+            .data)
+    }
 }
 
 /// Builder for migration parameters.
-- 
2.47.3





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

* [RFC PATCH datacenter-manager 2/3] ui: acl: list granular level permission paths for views
  2026-03-25 15:35 [RFC PATCH datacenter-manager 0/3] ui: acl: pre-populate permission path selector Shan Shaji
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 1/3] pdm-client: add `list_views` function to fetch views list Shan Shaji
@ 2026-03-25 15:35 ` Shan Shaji
  2026-03-26 11:16   ` Shannon Sterz
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources Shan Shaji
  2 siblings, 1 reply; 8+ messages in thread
From: Shan Shaji @ 2026-03-25 15:35 UTC (permalink / raw)
  To: pdm-devel

Previously, users were unable to assign permissions to individual views.
Resolved this by loading the views list and extending permission paths
to include specific view routes.

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 .../configuration/permission_path_selector.rs | 80 +++++++++++++++----
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
index 59bdabb..a0fec86 100644
--- a/ui/src/configuration/permission_path_selector.rs
+++ b/ui/src/configuration/permission_path_selector.rs
@@ -1,12 +1,15 @@
 use std::rc::Rc;
 
+use anyhow::Error;
 use yew::html::IntoPropValue;
 
-use pwt::prelude::*;
 use pwt::widget::form::Combobox;
+use pwt::{prelude::*, AsyncPool};
 
 use pwt_macros::{builder, widget};
 
+use crate::pdm_client;
+
 static PREDEFINED_PATHS: &[&str] = &[
     "/",
     "/access",
@@ -44,33 +47,78 @@ impl PermissionPathSelector {
     }
 }
 
-enum Msg {}
+enum Msg {
+    Prefetched(Vec<String>),
+    PrefetchFailed,
+}
 
 struct PdmPermissionPathSelector {
     items: Rc<Vec<AttrValue>>,
+    async_pool: AsyncPool,
 }
 
-impl PdmPermissionPathSelector {}
+impl PdmPermissionPathSelector {
+    async fn get_view_paths() -> Result<Vec<String>, Error> {
+        let views = pdm_client().list_views().await?;
+        let paths: Vec<String> = views
+            .iter()
+            .map(|cfg| format!("/view/{}", cfg.id))
+            .collect();
+        Ok(paths)
+    }
+
+    async fn get_paths() -> Result<Vec<String>, Error> {
+        let paths = Self::get_view_paths().await?;
+        Ok(paths)
+    }
+}
 
 impl Component for PdmPermissionPathSelector {
     type Message = Msg;
     type Properties = PermissionPathSelector;
 
-    fn create(_ctx: &Context<Self>) -> Self {
-        // TODO: fetch resources & remotes from the backend to improve the pre-defined selection of
-        // acl paths
-        Self {
-            items: Rc::new(
-                PREDEFINED_PATHS
-                    .iter()
-                    .map(|i| AttrValue::from(*i))
-                    .collect(),
-            ),
-        }
+    fn create(ctx: &Context<Self>) -> Self {
+        let base_items: Vec<AttrValue> = PREDEFINED_PATHS
+            .iter()
+            .map(|i| AttrValue::from(*i))
+            .collect();
+
+        let this = Self {
+            items: Rc::new(base_items),
+            async_pool: AsyncPool::new(),
+        };
+
+        let link = ctx.link().clone();
+        this.async_pool.spawn(async move {
+            let paths = Self::get_paths().await;
+            match paths {
+                Ok(paths) => {
+                    link.send_message(Msg::Prefetched(paths));
+                }
+                Err(_) => {
+                    link.send_message(Msg::PrefetchFailed);
+                }
+            }
+        });
+
+        this
     }
 
-    fn update(&mut self, _ctx: &Context<Self>, _msg: Self::Message) -> bool {
-        false
+    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
+        match msg {
+            Msg::Prefetched(paths) => {
+                let mut items: Vec<String> = Vec::with_capacity(self.items.len() + paths.len());
+
+                items.extend(self.items.iter().map(|item| item.to_string()));
+                items.extend(paths.into_iter());
+                items.sort_by(|a, b| a.to_lowercase().cmp(&b.to_lowercase()));
+
+                self.items = Rc::new(items.into_iter().map(AttrValue::from).collect());
+
+                true
+            }
+            Msg::PrefetchFailed => false,
+        }
     }
 
     fn view(&self, ctx: &Context<Self>) -> Html {
-- 
2.47.3





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

* [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources
  2026-03-25 15:35 [RFC PATCH datacenter-manager 0/3] ui: acl: pre-populate permission path selector Shan Shaji
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 1/3] pdm-client: add `list_views` function to fetch views list Shan Shaji
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 2/3] ui: acl: list granular level permission paths for views Shan Shaji
@ 2026-03-25 15:35 ` Shan Shaji
  2026-03-26 11:16   ` Shannon Sterz
  2026-03-27 10:21   ` Lukas Wagner
  2 siblings, 2 replies; 8+ messages in thread
From: Shan Shaji @ 2026-03-25 15:35 UTC (permalink / raw)
  To: pdm-devel

previously, users were not able to assign permissions to specific
resources. Resolved this by listing the resources Ids and extending the
permission paths to include specific resource paths.

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 lib/pdm-api-types/src/acl.rs                  |  3 +-
 .../configuration/permission_path_selector.rs | 47 ++++++++++++++++++-
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/lib/pdm-api-types/src/acl.rs b/lib/pdm-api-types/src/acl.rs
index 405982a..825fd6b 100644
--- a/lib/pdm-api-types/src/acl.rs
+++ b/lib/pdm-api-types/src/acl.rs
@@ -280,9 +280,10 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
                 if components_len <= 2 {
                     return Ok(());
                 }
+
                 // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
                 match components[2] {
-                    "guest" | "storage" => {
+                    "guest" | "storage" | "datastore" | "node" => {
                         // /resource/{remote-id}/{resource-type}
                         // /resource/{remote-id}/{resource-type}/{resource-id}
                         if components_len <= 4 {
diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
index a0fec86..899fa05 100644
--- a/ui/src/configuration/permission_path_selector.rs
+++ b/ui/src/configuration/permission_path_selector.rs
@@ -9,6 +9,7 @@ use pwt::{prelude::*, AsyncPool};
 use pwt_macros::{builder, widget};
 
 use crate::pdm_client;
+use pdm_api_types::resource::ResourceType;
 
 static PREDEFINED_PATHS: &[&str] = &[
     "/",
@@ -67,8 +68,52 @@ impl PdmPermissionPathSelector {
         Ok(paths)
     }
 
+    async fn get_resource_paths() -> Result<Vec<String>, Error> {
+        let resources = pdm_client().resources(None, None).await?;
+        let resource_paths: Vec<String> = resources
+            .into_iter()
+            .flat_map(|remote| {
+                let remote_name = remote.remote;
+
+                let mut base_paths = vec![remote_name.clone(), format!("{remote_name}/node")];
+                let is_pve = remote.resources.iter().any(|r| {
+                    matches!(
+                        r.resource_type(),
+                        ResourceType::PveQemu | ResourceType::PveLxc
+                    )
+                });
+
+                if is_pve {
+                    base_paths.push(format!("{remote_name}/guest"));
+                }
+
+                let resource_ids =
+                    remote.resources.into_iter().filter_map(|resource| {
+                        match resource.resource_type() {
+                            ResourceType::PveLxc
+                            | ResourceType::PveQemu
+                            | ResourceType::Node
+                            | ResourceType::PbsDatastore => resource
+                                .global_id()
+                                .strip_prefix("remote/")
+                                .map(str::to_owned),
+                            _ => None,
+                        }
+                    });
+
+                base_paths.into_iter().chain(resource_ids)
+            })
+            .map(|v| format!("{}{v}", "/resource/"))
+            .collect();
+        Ok(resource_paths)
+    }
+
     async fn get_paths() -> Result<Vec<String>, Error> {
-        let paths = Self::get_view_paths().await?;
+        let mut paths = Self::get_view_paths().await?;
+        let mut resource_paths = Self::get_resource_paths().await?;
+
+        paths.append(&mut resource_paths);
+
         Ok(paths)
     }
 }
-- 
2.47.3





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

* Re: [RFC PATCH datacenter-manager 2/3] ui: acl: list granular level permission paths for views
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 2/3] ui: acl: list granular level permission paths for views Shan Shaji
@ 2026-03-26 11:16   ` Shannon Sterz
  0 siblings, 0 replies; 8+ messages in thread
From: Shannon Sterz @ 2026-03-26 11:16 UTC (permalink / raw)
  To: Shan Shaji; +Cc: pdm-devel

some comments in-line.

On Wed Mar 25, 2026 at 4:35 PM CET, Shan Shaji wrote:
> Previously, users were unable to assign permissions to individual views.
> Resolved this by loading the views list and extending permission paths
> to include specific view routes.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  .../configuration/permission_path_selector.rs | 80 +++++++++++++++----
>  1 file changed, 64 insertions(+), 16 deletions(-)
>
> diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
> index 59bdabb..a0fec86 100644
> --- a/ui/src/configuration/permission_path_selector.rs
> +++ b/ui/src/configuration/permission_path_selector.rs
> @@ -1,12 +1,15 @@
>  use std::rc::Rc;
>
> +use anyhow::Error;
>  use yew::html::IntoPropValue;
>
> -use pwt::prelude::*;
>  use pwt::widget::form::Combobox;
> +use pwt::{prelude::*, AsyncPool};
>
>  use pwt_macros::{builder, widget};
>
> +use crate::pdm_client;
> +
>  static PREDEFINED_PATHS: &[&str] = &[
>      "/",
>      "/access",
> @@ -44,33 +47,78 @@ impl PermissionPathSelector {
>      }
>  }
>
> -enum Msg {}
> +enum Msg {
> +    Prefetched(Vec<String>),
> +    PrefetchFailed,
> +}
>
>  struct PdmPermissionPathSelector {
>      items: Rc<Vec<AttrValue>>,
> +    async_pool: AsyncPool,
>  }
>
> -impl PdmPermissionPathSelector {}
> +impl PdmPermissionPathSelector {
> +    async fn get_view_paths() -> Result<Vec<String>, Error> {
> +        let views = pdm_client().list_views().await?;
> +        let paths: Vec<String> = views
> +            .iter()
> +            .map(|cfg| format!("/view/{}", cfg.id))
> +            .collect();
> +        Ok(paths)
> +    }
> +
> +    async fn get_paths() -> Result<Vec<String>, Error> {
> +        let paths = Self::get_view_paths().await?;
> +        Ok(paths)
> +    }
> +}
>
>  impl Component for PdmPermissionPathSelector {
>      type Message = Msg;
>      type Properties = PermissionPathSelector;
>
> -    fn create(_ctx: &Context<Self>) -> Self {
> -        // TODO: fetch resources & remotes from the backend to improve the pre-defined selection of
> -        // acl paths
> -        Self {
> -            items: Rc::new(
> -                PREDEFINED_PATHS
> -                    .iter()
> -                    .map(|i| AttrValue::from(*i))
> -                    .collect(),
> -            ),
> -        }
> +    fn create(ctx: &Context<Self>) -> Self {
> +        let base_items: Vec<AttrValue> = PREDEFINED_PATHS
> +            .iter()
> +            .map(|i| AttrValue::from(*i))
> +            .collect();
> +
> +        let this = Self {
> +            items: Rc::new(base_items),
> +            async_pool: AsyncPool::new(),
> +        };
> +
> +        let link = ctx.link().clone();
> +        this.async_pool.spawn(async move {
> +            let paths = Self::get_paths().await;
> +            match paths {
> +                Ok(paths) => {
> +                    link.send_message(Msg::Prefetched(paths));
> +                }
> +                Err(_) => {
> +                    link.send_message(Msg::PrefetchFailed);
> +                }

this is minor, but at least for me cargo fmt will also accept the
following formatting:

            match paths {
                Ok(paths) => link.send_message(Msg::Prefetched(paths)),
                Err(_) => link.send_message(Msg::PrefetchFailed),
            }

imo that's a little neater.

> +            }
> +        });

im generally not too big of a fan of this "this" pattern in rust and
here there is no need for it, you can simply do:

    let async_pool = AsyncPool::new();
    async_pool.spawn(..);
    Self {
        items: Rc::new(..),
        async_pool,
    }

you may need to change `async_pool` in the struct to `_async_pool`
because then this member is never directly accessed.

> +
> +        this
>      }
>
> -    fn update(&mut self, _ctx: &Context<Self>, _msg: Self::Message) -> bool {
> -        false
> +    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
> +        match msg {
> +            Msg::Prefetched(paths) => {
> +                let mut items: Vec<String> = Vec::with_capacity(self.items.len() + paths.len());
> +
> +                items.extend(self.items.iter().map(|item| item.to_string()));
> +                items.extend(paths.into_iter());
> +                items.sort_by(|a, b| a.to_lowercase().cmp(&b.to_lowercase()));
> +
> +                self.items = Rc::new(items.into_iter().map(AttrValue::from).collect());

this could be expressed more neatly as:

```
            Msg::Prefetched(paths) => {
                let items = Rc::make_mut(&mut self.items);
                items.extend(paths.into_iter().map(AttrValue::from));
                items.sort_by_key(|k| k.to_lowercase());
                true
            }
```

this has a couple of advantages:

* we don't move the original items from `AttrValue` to a `String` back
  to `AttrValue`, saving on allocations.
* `Rc::make_mut()` will only clone if there are other pointers to the
  same allocation. since the is an `Rc::clone` in the view method below,
  this probably won't actually impact much here from what i can tell,
  but it still seems sensible to me to only force a new `Rc<Vec>` if
  absolutely needed.
* uses `sort_by_key` instead of `sort_by` is more concise and also
  recommended by clippy.

> +
> +                true
> +            }
> +            Msg::PrefetchFailed => false,
> +        }
>      }
>
>      fn view(&self, ctx: &Context<Self>) -> Html {





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

* Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources Shan Shaji
@ 2026-03-26 11:16   ` Shannon Sterz
  2026-03-26 13:58     ` Shan Shaji
  2026-03-27 10:21   ` Lukas Wagner
  1 sibling, 1 reply; 8+ messages in thread
From: Shannon Sterz @ 2026-03-26 11:16 UTC (permalink / raw)
  To: Shan Shaji; +Cc: pdm-devel

some comments in-line.

On Wed Mar 25, 2026 at 4:35 PM CET, Shan Shaji wrote:
> previously, users were not able to assign permissions to specific
> resources. Resolved this by listing the resources Ids and extending the
> permission paths to include specific resource paths.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  lib/pdm-api-types/src/acl.rs                  |  3 +-
>  .../configuration/permission_path_selector.rs | 47 ++++++++++++++++++-
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/lib/pdm-api-types/src/acl.rs b/lib/pdm-api-types/src/acl.rs
> index 405982a..825fd6b 100644
> --- a/lib/pdm-api-types/src/acl.rs
> +++ b/lib/pdm-api-types/src/acl.rs
> @@ -280,9 +280,10 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
>                  if components_len <= 2 {
>                      return Ok(());
>                  }
> +
>                  // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
>                  match components[2] {
> -                    "guest" | "storage" => {
> +                    "guest" | "storage" | "datastore" | "node" => {

while this seems minor, i think there is a case to be made that this
should be split out into its own patch. changing the validation here
changes what ACLs are allowed in general in PDM. this should probably
receive more attention than to be folded into a patch that focuses on UI
functionality otherwise.

>                          // /resource/{remote-id}/{resource-type}
>                          // /resource/{remote-id}/{resource-type}/{resource-id}
>                          if components_len <= 4 {
> diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
> index a0fec86..899fa05 100644
> --- a/ui/src/configuration/permission_path_selector.rs
> +++ b/ui/src/configuration/permission_path_selector.rs
> @@ -9,6 +9,7 @@ use pwt::{prelude::*, AsyncPool};
>  use pwt_macros::{builder, widget};
>
>  use crate::pdm_client;
> +use pdm_api_types::resource::ResourceType;
>
>  static PREDEFINED_PATHS: &[&str] = &[
>      "/",
> @@ -67,8 +68,52 @@ impl PdmPermissionPathSelector {
>          Ok(paths)
>      }
>
> +    async fn get_resource_paths() -> Result<Vec<String>, Error> {
> +        let resources = pdm_client().resources(None, None).await?;
> +        let resource_paths: Vec<String> = resources
> +            .into_iter()
> +            .flat_map(|remote| {
> +                let remote_name = remote.remote;
> +
> +                let mut base_paths = vec![remote_name.clone(), format!("{remote_name}/node")];
> +                let is_pve = remote.resources.iter().any(|r| {
> +                    matches!(
> +                        r.resource_type(),
> +                        ResourceType::PveQemu | ResourceType::PveLxc
> +                    )

correct me if im wrong, but if a pve remote has no guests, than this
would not detect it as a pve remote and not add the
`{remote_name}/guest" acl path here? im not sure that is sensible, users
may want to grant this more general acl *before* guests are added to a
specific remote.

looking through the code here, it seems querying the type of remote
would require at least one more api call (list_remotes). instead you
should be able to get a list of remotes from the context though. we
provide one in the top level main_menu iirc. you should be able to query
it with `ctx.link().context(Callback::from(|_: RemoteList|{}))` or
similar. that should give you better type information on the remotes.
can you try that?

> +                });
> +
> +                if is_pve {
> +                    base_paths.push(format!("{remote_name}/guest"));
> +                }
> +
> +                let resource_ids =
> +                    remote.resources.into_iter().filter_map(|resource| {
> +                        match resource.resource_type() {
> +                            ResourceType::PveLxc
> +                            | ResourceType::PveQemu
> +                            | ResourceType::Node
> +                            | ResourceType::PbsDatastore => resource
> +                                .global_id()
> +                                .strip_prefix("remote/")
> +                                .map(str::to_owned),
> +                            _ => None,
> +                        }
> +                    });
> +
> +                base_paths.into_iter().chain(resource_ids)
> +            })
> +            .map(|v| format!("{}{v}", "/resource/"))

this should be `format!("/resource/{v}")`

> +            .collect();
> +        Ok(resource_paths)
> +    }
> +
>      async fn get_paths() -> Result<Vec<String>, Error> {
> -        let paths = Self::get_view_paths().await?;
> +        let mut paths = Self::get_view_paths().await?;
> +        let mut resource_paths = Self::get_resource_paths().await?;
> +
> +        paths.append(&mut resource_paths);
> +
>          Ok(paths)
>      }
>  }





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

* Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources
  2026-03-26 11:16   ` Shannon Sterz
@ 2026-03-26 13:58     ` Shan Shaji
  0 siblings, 0 replies; 8+ messages in thread
From: Shan Shaji @ 2026-03-26 13:58 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pdm-devel

On Thu Mar 26, 2026 at 12:16 PM CET, Shannon Sterz wrote:
> some comments in-line.
>
> On Wed Mar 25, 2026 at 4:35 PM CET, Shan Shaji wrote:
>> previously, users were not able to assign permissions to specific
>> resources. Resolved this by listing the resources Ids and extending the
>> permission paths to include specific resource paths.
>>
>> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
>> ---
>>  lib/pdm-api-types/src/acl.rs                  |  3 +-
>>  .../configuration/permission_path_selector.rs | 47 ++++++++++++++++++-
>>  2 files changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/pdm-api-types/src/acl.rs b/lib/pdm-api-types/src/acl.rs
>> index 405982a..825fd6b 100644
>> --- a/lib/pdm-api-types/src/acl.rs
>> +++ b/lib/pdm-api-types/src/acl.rs
>> @@ -280,9 +280,10 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
>>                  if components_len <= 2 {
>>                      return Ok(());
>>                  }
>> +
>>                  // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
>>                  match components[2] {
>> -                    "guest" | "storage" => {
>> +                    "guest" | "storage" | "datastore" | "node" => {
>
> while this seems minor, i think there is a case to be made that this
> should be split out into its own patch. changing the validation here
> changes what ACLs are allowed in general in PDM. this should probably
> receive more attention than to be folded into a patch that focuses on UI
> functionality otherwise.

Will seperate it into it's own patch. Thank you!

>>                          // /resource/{remote-id}/{resource-type}
>>                          // /resource/{remote-id}/{resource-type}/{resource-id}
>>                          if components_len <= 4 {
>> diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
>> index a0fec86..899fa05 100644
>> --- a/ui/src/configuration/permission_path_selector.rs
>> +++ b/ui/src/configuration/permission_path_selector.rs
>> @@ -9,6 +9,7 @@ use pwt::{prelude::*, AsyncPool};
>>  use pwt_macros::{builder, widget};
>>
>>  use crate::pdm_client;
>> +use pdm_api_types::resource::ResourceType;
>>
>>  static PREDEFINED_PATHS: &[&str] = &[
>>      "/",
>> @@ -67,8 +68,52 @@ impl PdmPermissionPathSelector {
>>          Ok(paths)
>>      }
>>
>> +    async fn get_resource_paths() -> Result<Vec<String>, Error> {
>> +        let resources = pdm_client().resources(None, None).await?;
>> +        let resource_paths: Vec<String> = resources
>> +            .into_iter()
>> +            .flat_map(|remote| {
>> +                let remote_name = remote.remote;
>> +
>> +                let mut base_paths = vec![remote_name.clone(), format!("{remote_name}/node")];
>> +                let is_pve = remote.resources.iter().any(|r| {
>> +                    matches!(
>> +                        r.resource_type(),
>> +                        ResourceType::PveQemu | ResourceType::PveLxc
>> +                    )
>
> correct me if im wrong, but if a pve remote has no guests, than this
> would not detect it as a pve remote and not add the
> `{remote_name}/guest" acl path here?

yes, you're right. The guest path will only be added if the remote resources
have any guest. 

> im not sure that is sensible, users
> may want to grant this more general acl *before* guests are added to a
> specific remote.

hmm.. that does makes sense. Will change this. Thank you!

> looking through the code here, it seems querying the type of remote
> would require at least one more api call (list_remotes). instead you
> should be able to get a list of remotes from the context though. we
> provide one in the top level main_menu iirc. you should be able to query
> it with `ctx.link().context(Callback::from(|_: RemoteList|{}))` or
> similar. that should give you better type information on the remotes.
> can you try that?

Sure, will try that. Thank You! 

>> +                });
>> +
>> +                if is_pve {
>> +                    base_paths.push(format!("{remote_name}/guest"));
>> +                }
>> +
>> +                let resource_ids =
>> +                    remote.resources.into_iter().filter_map(|resource| {
>> +                        match resource.resource_type() {
>> +                            ResourceType::PveLxc
>> +                            | ResourceType::PveQemu
>> +                            | ResourceType::Node
>> +                            | ResourceType::PbsDatastore => resource
>> +                                .global_id()
>> +                                .strip_prefix("remote/")
>> +                                .map(str::to_owned),
>> +                            _ => None,
>> +                        }
>> +                    });
>> +
>> +                base_paths.into_iter().chain(resource_ids)
>> +            })
>> +            .map(|v| format!("{}{v}", "/resource/"))
>
> this should be `format!("/resource/{v}")`

Will update it.

>> +            .collect();
>> +        Ok(resource_paths)
>> +    }
>> +
>>      async fn get_paths() -> Result<Vec<String>, Error> {
>> -        let paths = Self::get_view_paths().await?;
>> +        let mut paths = Self::get_view_paths().await?;
>> +        let mut resource_paths = Self::get_resource_paths().await?;
>> +
>> +        paths.append(&mut resource_paths);
>> +
>>          Ok(paths)
>>      }
>>  }





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

* Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources
  2026-03-25 15:35 ` [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources Shan Shaji
  2026-03-26 11:16   ` Shannon Sterz
@ 2026-03-27 10:21   ` Lukas Wagner
  1 sibling, 0 replies; 8+ messages in thread
From: Lukas Wagner @ 2026-03-27 10:21 UTC (permalink / raw)
  To: Shan Shaji, pdm-devel

On Wed Mar 25, 2026 at 4:35 PM CET, Shan Shaji wrote:
> previously, users were not able to assign permissions to specific
> resources. Resolved this by listing the resources Ids and extending the
> permission paths to include specific resource paths.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  lib/pdm-api-types/src/acl.rs                  |  3 +-
>  .../configuration/permission_path_selector.rs | 47 ++++++++++++++++++-
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/lib/pdm-api-types/src/acl.rs b/lib/pdm-api-types/src/acl.rs
> index 405982a..825fd6b 100644
> --- a/lib/pdm-api-types/src/acl.rs
> +++ b/lib/pdm-api-types/src/acl.rs
> @@ -280,9 +280,10 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
>                  if components_len <= 2 {
>                      return Ok(());
>                  }
> +
>                  // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
>                  match components[2] {
> -                    "guest" | "storage" => {
> +                    "guest" | "storage" | "datastore" | "node" => {
>                          // /resource/{remote-id}/{resource-type}
>                          // /resource/{remote-id}/{resource-type}/{resource-id}
>                          if components_len <= 4 {

Looks correct to me (quickly double-checked the code base), but as
Shannon already mentioned, this should be its own patch.


> diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
> index a0fec86..899fa05 100644
> --- a/ui/src/configuration/permission_path_selector.rs
> +++ b/ui/src/configuration/permission_path_selector.rs
> @@ -9,6 +9,7 @@ use pwt::{prelude::*, AsyncPool};
>  use pwt_macros::{builder, widget};
>  
>  use crate::pdm_client;
> +use pdm_api_types::resource::ResourceType;
>  
>  static PREDEFINED_PATHS: &[&str] = &[
>      "/",
> @@ -67,8 +68,52 @@ impl PdmPermissionPathSelector {
>          Ok(paths)
>      }
>  
> +    async fn get_resource_paths() -> Result<Vec<String>, Error> {
> +        let resources = pdm_client().resources(None, None).await?;
> +        let resource_paths: Vec<String> = resources
> +            .into_iter()
> +            .flat_map(|remote| {
> +                let remote_name = remote.remote;
> +
> +                let mut base_paths = vec![remote_name.clone(), format!("{remote_name}/node")];
> +                let is_pve = remote.resources.iter().any(|r| {
> +                    matches!(
> +                        r.resource_type(),
> +                        ResourceType::PveQemu | ResourceType::PveLxc
> +                    )
> +                });
> +
> +                if is_pve {
> +                    base_paths.push(format!("{remote_name}/guest"));
> +                }
> +
> +                let resource_ids =
> +                    remote.resources.into_iter().filter_map(|resource| {
> +                        match resource.resource_type() {
> +                            ResourceType::PveLxc
> +                            | ResourceType::PveQemu
> +                            | ResourceType::Node
> +                            | ResourceType::PbsDatastore => resource
> +                                .global_id()
> +                                .strip_prefix("remote/")
> +                                .map(str::to_owned),
> +                            _ => None,
> +                        }
> +                    });
> +
> +                base_paths.into_iter().chain(resource_ids)
> +            })
> +            .map(|v| format!("{}{v}", "/resource/"))
> +            .collect();
> +        Ok(resource_paths)
> +    }
> +

I don't think we should pre-populate the ACL path selector with *all*
possible resource ACL paths. If you have a big setup with a large number
of remotes/resources, the list can quickly have thousands of entries.
While the performance seemed acceptable in my first test with roughly
6000 resources (which still is not *that* of a large setup), I don't
think having such a huge list of options is a good experience for the
user.

I think it might be best to just add entries for each remote (so
/resource/<remote>) for now.

If we ever want to provide more granularity, I think we need a smarter
approach for selecting/narrowing down on the ACL object in this dialog.


>      async fn get_paths() -> Result<Vec<String>, Error> {
> -        let paths = Self::get_view_paths().await?;
> +        let mut paths = Self::get_view_paths().await?;
> +        let mut resource_paths = Self::get_resource_paths().await?;
> +
> +        paths.append(&mut resource_paths);
> +
>          Ok(paths)
>      }
>  }





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

end of thread, other threads:[~2026-03-27 10:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-25 15:35 [RFC PATCH datacenter-manager 0/3] ui: acl: pre-populate permission path selector Shan Shaji
2026-03-25 15:35 ` [RFC PATCH datacenter-manager 1/3] pdm-client: add `list_views` function to fetch views list Shan Shaji
2026-03-25 15:35 ` [RFC PATCH datacenter-manager 2/3] ui: acl: list granular level permission paths for views Shan Shaji
2026-03-26 11:16   ` Shannon Sterz
2026-03-25 15:35 ` [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources Shan Shaji
2026-03-26 11:16   ` Shannon Sterz
2026-03-26 13:58     ` Shan Shaji
2026-03-27 10:21   ` Lukas Wagner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal