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

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

On Fri Mar 27, 2026 at 11:21 AM CET, Lukas Wagner wrote:
> 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.

totally agree. Shannon also mentioned the same concern (offlist). Also if there
are too many resources the dropdown is really too big. We will need to improve
the widget to show the list as scrollable content. But i believe that needs
to be another series.

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

Then for now, In this series (v1) I will populate the paths with remote and
view names. 

> 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.

AFAICT, In our documentation [0] we are already mentioning about the granular
permissions for resources. 

- [0] https://pdm.proxmox.com/docs/access-control.html#objects-and-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)
>>      }
>>  }