Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources

["Lukas Wagner" <l.wagner@proxmox.com>]

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)
>      }
>  }