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 6C5DD1FF13F for ; Thu, 26 Mar 2026 14:58:13 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 65C5B1441F; Thu, 26 Mar 2026 14:58:34 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 26 Mar 2026 14:58:00 +0100 Message-Id: Subject: Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources From: "Shan Shaji" To: "Shannon Sterz" 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: 1774533431620 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.025 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 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: THBKZNANK2U37WX57TBW6THHHATA4Y5J X-Message-ID-Hash: THBKZNANK2U37WX57TBW6THHHATA4Y5J 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 CC: pdm-devel@lists.proxmox.com 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 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 >> --- >> 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> { > > 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}/{resou= rce-id} >> if components_len <=3D 4 { >> 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}; >> >> use crate::pdm_client; >> +use pdm_api_types::resource::ResourceType; >> >> static PREDEFINED_PATHS: &[&str] =3D &[ >> "/", >> @@ -67,8 +68,52 @@ impl PdmPermissionPathSelector { >> Ok(paths) >> } >> >> + 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 >> + ) > > 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 resource= s have any guest.=20 > 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!=20 >> + }); >> + >> + 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/")) > > this should be `format!("/resource/{v}")` Will update it. >> + .collect(); >> + Ok(resource_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) >> } >> }