From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id A31691FF13F for ; Thu, 26 Mar 2026 12:16:19 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 11668F269; Thu, 26 Mar 2026 12:16:41 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 26 Mar 2026 12:16:06 +0100 Message-Id: Subject: Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources To: "Shan Shaji" X-Mailer: aerc 0.20.0 References: <20260325153535.286380-1-s.shaji@proxmox.com> <20260325153535.286380-4-s.shaji@proxmox.com> In-Reply-To: <20260325153535.286380-4-s.shaji@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774523718055 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.027 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: B27KS6HKLGTPMOWDBXAG2EMCJO2FMCKH X-Message-ID-Hash: B27KS6HKLGTPMOWDBXAG2EMCJO2FMCKH X-MailFrom: s.sterz@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: 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::AccessControlConf= ig for AccessControlConfig { > if components_len <=3D 2 { > return Ok(()); > } > + > // `/resource/{remote-id}/{resource-type=3Dguest,storage= }/{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. > // /resource/{remote-id}/{resource-type} > // /resource/{remote-id}/{resource-type}/{resour= ce-id} > if components_len <=3D 4 { > diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/co= nfiguration/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? im not sure that is sensible, users may want to grant this more general acl *before* guests are added to a specific remote. 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? > + }); > + > + 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}")` > + .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) > } > }