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

["Shan Shaji" <s.shaji@proxmox.com>]

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