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

Shan Shaji
2026-03-26T13:58:13Z

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