From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id EA2CC1FF141 for ; Mon, 30 Mar 2026 11:07:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C535F1B917; Mon, 30 Mar 2026 11:07:35 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 30 Mar 2026 11:07:28 +0200 Message-Id: Subject: Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources From: "Shan Shaji" To: "Lukas Wagner" , X-Mailer: aerc 0.20.0 References: <20260325153535.286380-1-s.shaji@proxmox.com> <20260325153535.286380-4-s.shaji@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774861594639 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.525 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: KR4IOSTRRXQPJVWELSA2BYHBPXQJYRU4 X-Message-ID-Hash: KR4IOSTRRXQPJVWELSA2BYHBPXQJYRU4 X-MailFrom: s.shaji@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 >> --- >> 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::AccessControlCon= fig for AccessControlConfig { >> if components_len <=3D 2 { >> return Ok(()); >> } >> + >> // `/resource/{remote-id}/{resource-type=3Dguest,storag= e}/{resource-id}` >> match components[2] { >> - "guest" | "storage" =3D> { >> + "guest" | "storage" | "datastore" | "node" =3D> { >> // /resource/{remote-id}/{resource-type} >> // /resource/{remote-id}/{resource-type}/{resou= rce-id} >> if components_len <=3D 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/c= onfiguration/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}; >> =20 >> use crate::pdm_client; >> +use pdm_api_types::resource::ResourceType; >> =20 >> static PREDEFINED_PATHS: &[&str] =3D &[ >> "/", >> @@ -67,8 +68,52 @@ impl PdmPermissionPathSelector { >> Ok(paths) >> } >> =20 >> + async fn get_resource_paths() -> Result, Error> { >> + let resources =3D pdm_client().resources(None, None).await?; >> + let resource_paths: Vec =3D resources >> + .into_iter() >> + .flat_map(|remote| { >> + let remote_name =3D remote.remote; >> + >> + let mut base_paths =3D vec![remote_name.clone(), format= !("{remote_name}/node")]; >> + let is_pve =3D 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 =3D >> + remote.resources.into_iter().filter_map(|resource| = { >> + match resource.resource_type() { >> + ResourceType::PveLxc >> + | ResourceType::PveQemu >> + | ResourceType::Node >> + | ResourceType::PbsDatastore =3D> resource >> + .global_id() >> + .strip_prefix("remote/") >> + .map(str::to_owned), >> + _ =3D> 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 t= here are too many resources the dropdown is really too big. We will need to impr= ove 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/) for now. Then for now, In this series (v1) I will populate the paths with remote and view names.=20 > 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 granul= ar permissions for resources.=20 - [0] https://pdm.proxmox.com/docs/access-control.html#objects-and-paths >> async fn get_paths() -> Result, Error> { >> - let paths =3D Self::get_view_paths().await?; >> + let mut paths =3D Self::get_view_paths().await?; >> + let mut resource_paths =3D Self::get_resource_paths().await?; >> + >> + paths.append(&mut resource_paths); >> + >> Ok(paths) >> } >> }