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 1C4D01FF165 for ; Thu, 23 Oct 2025 13:33:19 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 945F585E0; Thu, 23 Oct 2025 13:33:46 +0200 (CEST) Mime-Version: 1.0 Date: Thu, 23 Oct 2025 13:33:12 +0200 Message-Id: To: "Dominik Csapak" X-Mailer: aerc 0.20.0 References: <20251022131126.358790-1-s.sterz@proxmox.com> <20251022131126.358790-6-s.sterz@proxmox.com> <51892c50-105b-4793-ab51-f7507dd02117@proxmox.com> In-Reply-To: <51892c50-105b-4793-ab51-f7507dd02117@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761219184507 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.244 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 ENA_SUBJ_ODD_CASE 2.6 Subject has odd case KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH yew-comp v2 1/2] acl_context: add AclContext and AclContextProvider X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Thu Oct 23, 2025 at 12:00 PM CEST, Dominik Csapak wrote: > Code looks ok, but I think there might be an easier way to > achieve a similar result: > > Instead of having a new global callback that we update on each auth > change, couldn't we reverse that and have a "simple" component with > context that updates the tree when the auth client changes AFAICS we > already have a 'notify_auth_listener' so we could use that > (maybe we have to trigger that on each update, not sure) > > I think such an approach would be > 1. less code > 2. easier to follow > > what do you think? i mean the "simple" component would in my opinion just be the `AclContextProvider`. the problem is, and correct me if im wrong here, `notify_auth_listener gets passed to `HttpWasmClient` as its `on_auth_failure` callback (and isn't used anywhere else). that callback get called in two cases: - when the client did a `HttpWasmClient::fetch_request` and got a 401 (in that case with `false`) - when the client clears its authenticatio via `HttpWasmClient::clear_auth` (in that case with `true`) so it would only get notified when the client logs out. not when a user logs in or a ticket gets refreshed. so doing to do this properly, i'd like a `on_new_auth` callback or similar on HttpWasmClient, but that would be quite a bit more churn. not to mention, that we would still need to have the dynamic component register itself against the static client there. which would probably look more or less the same way this does. the advantage would be, that we can of course re-use the mechanism then in other cases. > > On 10/22/25 3:11 PM, Shannon Sterz wrote: >> these components allow an application to provide a context that >> compononets can use to check the privileges of the current user. thus, >> they can omit ui elements if the user lacks the permissions to use >> them. >> >> by using a context, all components that use it will get reactively >> re-rendered if the context changes. >> >> Signed-off-by: Shannon Sterz >> --- >> Cargo.toml | 2 +- >> src/acl_context.rs | 204 +++++++++++++++++++++++++++++++++++++++++++++ >> src/lib.rs | 3 + >> 3 files changed, 208 insertions(+), 1 deletion(-) >> create mode 100644 src/acl_context.rs >> >> diff --git a/Cargo.toml b/Cargo.toml >> index dc06ccc..520cb71 100644 >> --- a/Cargo.toml >> +++ b/Cargo.toml >> @@ -79,7 +79,7 @@ proxmox-auth-api = { version = "1", default-features = false, features = [ >> "api-types", >> ] } >> proxmox-apt-api-types = { version = "2.0", optional = true } >> -proxmox-access-control = "1.1" >> +proxmox-access-control = { version = "1.1", features = ["acl"]} >> proxmox-dns-api = { version = "1", optional = true } >> proxmox-network-api = { version = "1", optional = true } >> >> diff --git a/src/acl_context.rs b/src/acl_context.rs >> new file mode 100644 >> index 0000000..7cfa450 >> --- /dev/null >> +++ b/src/acl_context.rs >> @@ -0,0 +1,204 @@ >> +use std::cell::RefCell; >> +use std::rc::Rc; >> + >> +use serde::{Deserialize, Serialize}; >> +use yew::prelude::*; >> + >> +use proxmox_access_control::acl::AclTree; >> +use proxmox_access_control::types::{AclListItem, AclUgidType}; >> +use pwt::state::PersistentState; >> +use pwt::AsyncAbortGuard; >> + >> +use pbs_api_types::Authid; >> + >> +use crate::CLIENT; >> + >> +thread_local! { >> + // Set by the current `AclContextProvider`, only one `AclContextProvider` should be used at a >> + // time. `LocalAclTree::load()` will use this callback, if present, to inform the `AclContext` >> + // that a new `AclTree` has been loaded. If the tree is different from the previously used >> + // tree, all components using the `AclContext` will be re-rendered with the new information. >> + static ACL_TREE_UPDATE_CB: Rc>>>> = Rc::new(RefCell::new(None)); >> +} >> + >> +#[derive(Clone)] >> +pub struct AclContext { >> + acl_tree: UseReducerHandle, >> + _abort_guard: Rc, >> +} >> + >> +impl AclContext { >> + /// Allows checking whether a users has sufficient privileges for a given ACL path. >> + /// >> + /// # Panics >> + /// >> + /// Requires that the access control configuration is initialized via >> + /// `proxmox_access_control::init::init_access_config` and will panic otherwise. >> + pub fn check_privs(&self, path: &[&str], required_privs: u64) -> bool { >> + self.acl_tree.check_privs(path, required_privs) >> + } >> + >> + /// Allows checking whether a user has any of the specified privileges under a certain ACL path. >> + /// >> + /// # Panics >> + /// >> + /// Requires that the access control configuration is initialized via >> + /// `proxmox_access_control::init::init_access_config` and will panic otherwise. >> + pub fn any_privs_below(&self, path: &[&str], required_privs: u64) -> bool { >> + self.acl_tree.any_privs_below(path, required_privs) >> + } >> +} >> + >> +// Needed for yew to determine whether components using the context need re-rendering. Only the >> +// AclTree matters here, so ignore the other fields. >> +impl PartialEq for AclContext { >> + fn eq(&self, other: &Self) -> bool { >> + self.acl_tree.eq(&other.acl_tree) >> + } >> +} >> + >> +#[derive(Properties, Debug, PartialEq)] >> +pub struct AclContextProviderProps { >> + #[prop_or_default] >> + pub children: Html, >> +} >> + >> +#[function_component] >> +pub fn AclContextProvider(props: &AclContextProviderProps) -> Html { >> + let reduce_handle = use_reducer_eq(LocalAclTree::new); >> + let acl_tree = reduce_handle.clone(); >> + >> + ACL_TREE_UPDATE_CB.with(|cb| { >> + cb.replace(Some(Callback::from(move |tree: Rc| { >> + reduce_handle.dispatch(tree); >> + }))); >> + }); >> + >> + let context = AclContext { >> + acl_tree, >> + _abort_guard: Rc::new(AsyncAbortGuard::spawn( >> + async move { LocalAclTree::load().await }, >> + )), >> + }; >> + >> + html!( >> + context={context} > >> + {props.children.clone()} >> + > >> + ) >> +} >> + >> +#[derive(Clone, PartialEq)] >> +pub(crate) struct LocalAclTree { >> + acl_tree: Rc, >> +} >> + >> +impl LocalAclTree { >> + const LOCAL_KEY: &str = "ProxmoxLocalAclTree"; >> + >> + /// Create a new `LocalAclTree` from the local storage. If no previous tree was persisted, an >> + /// empty tree will be used by default. >> + fn new() -> Self { >> + let saved_tree: PersistentState = PersistentState::new(Self::LOCAL_KEY); >> + >> + LocalAclTree { >> + acl_tree: Rc::new((&saved_tree.into_inner()).into()), >> + } >> + } >> + >> + fn check_privs(&self, path: &[&str], required_privs: u64) -> bool { >> + let Some(auth_id) = Self::get_current_authid() else { >> + log::error!("Could not get current user's authid, cannot check permissions."); >> + return false; >> + }; >> + >> + self.acl_tree >> + .check_privs(&auth_id, path, required_privs, true) >> + .is_ok() >> + } >> + >> + fn any_privs_below(&self, path: &[&str], required_privs: u64) -> bool { >> + let Some(auth_id) = Self::get_current_authid() else { >> + log::error!("Could not get current user's authid, cannot check permissions."); >> + return false; >> + }; >> + >> + self.acl_tree >> + .any_privs_below(&auth_id, path, required_privs) >> + .unwrap_or_default() >> + } >> + >> + /// Loads the currently logged in user's ACL list entries and assembles a local ACL tree. On >> + /// successful load, a copy will be persisted to local storage. If `ACL_TREE_UPDATE_CB` >> + /// contains a callback, it will be used to update the current `AclContext`. >> + pub(crate) async fn load() { >> + let Some(authid) = Self::get_current_authid() else { >> + log::error!("Could not get current Authid, please login first."); >> + return; >> + }; >> + >> + let nodes: Vec = >> + match crate::http_get("/access/acl?exact-authid=true", None).await { >> + Ok(nodes) => nodes, >> + Err(e) => { >> + log::error!("Could not load acl tree - {e:#}"); >> + return; >> + } >> + }; >> + >> + let to_save = SavedAclNodes { >> + authid: Some(authid), >> + nodes, >> + }; >> + >> + if let Some(ref cb) = *ACL_TREE_UPDATE_CB.with(|t| t.clone()).borrow() { >> + cb.emit(Rc::new((&to_save).into())); >> + } >> + >> + let mut saved_tree: PersistentState = PersistentState::new(Self::LOCAL_KEY); >> + saved_tree.update(to_save); >> + } >> + >> + fn get_current_authid() -> Option { >> + let authid = CLIENT.with_borrow(|t| t.get_auth())?; >> + authid.userid.parse::().ok() >> + } >> +} >> + >> +impl Reducible for LocalAclTree { >> + type Action = Rc; >> + >> + fn reduce(self: Rc, action: Self::Action) -> Rc { >> + Rc::new(Self { acl_tree: action }) >> + } >> +} >> + >> +#[derive(Deserialize, Serialize, PartialEq, Clone, Default)] >> +struct SavedAclNodes { >> + authid: Option, >> + nodes: Vec, >> +} >> + >> +impl From<&SavedAclNodes> for AclTree { >> + fn from(value: &SavedAclNodes) -> Self { >> + let mut tree = AclTree::new(); >> + >> + if let Some(ref authid) = value.authid { >> + for entry in &value.nodes { >> + match entry.ugid_type { >> + AclUgidType::User => { >> + tree.insert_user_role(&entry.path, authid, &entry.roleid, entry.propagate) >> + } >> + AclUgidType::Group => tree.insert_group_role( >> + &entry.path, >> + &entry.ugid, >> + &entry.roleid, >> + entry.propagate, >> + ), >> + } >> + } >> + } >> + >> + tree >> + } >> +} >> diff --git a/src/lib.rs b/src/lib.rs >> index 85e2b60..a0b5772 100644 >> --- a/src/lib.rs >> +++ b/src/lib.rs >> @@ -1,5 +1,8 @@ >> pub mod acme; >> >> +mod acl_context; >> +pub use acl_context::{AclContext, AclContextProvider}; >> + >> mod api_load_callback; >> pub use api_load_callback::{ApiLoadCallback, IntoApiLoadCallback}; >> _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel