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 80D641FF165 for ; Thu, 23 Oct 2025 13:39:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3CCBC8843; Thu, 23 Oct 2025 13:40:26 +0200 (CEST) Message-ID: <2a45c546-b7b3-4653-aef8-d27430427960@proxmox.com> Date: Thu, 23 Oct 2025 13:39:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Shannon Sterz References: <20251022131126.358790-1-s.sterz@proxmox.com> <20251022131126.358790-6-s.sterz@proxmox.com> <51892c50-105b-4793-ab51-f7507dd02117@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761219583792 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.273 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 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 0.001 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 0.001 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 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On 10/23/25 1:33 PM, Shannon Sterz wrote: > 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. that's what i meant with 'maybe we have to trigger that on each update' i don't see how that would add more churn, since we could 'just' add a parameter for the notification to determine what happened (logout, 401, reauth, etc.) and we'd have to trigger it in the places where you'd now do a LocalAclTree::load(), or am I missing something here? > > 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. we would just have to do a 'register_auth_observer' in the component once ? what i meant is that we can maybe reuse the auth observers not only for logout but for reauth too? > >> >> 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