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

["Shannon Sterz" <s.sterz@proxmox.com>]

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