Re: [pdm-devel] [PATCH yew-comp v2 1/2] acl_context: add AclContext and AclContextProvider

["Shannon Sterz" <s.sterz@proxmox.com>]

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 <s.sterz@proxmox.com>
>> ---
>>   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<RefCell<Option<Callback<Rc<AclTree>>>>> = Rc::new(RefCell::new(None));
>> +}
>> +
>> +#[derive(Clone)]
>> +pub struct AclContext {
>> +    acl_tree: UseReducerHandle<LocalAclTree>,
>> +    _abort_guard: Rc<AsyncAbortGuard>,
>> +}
>> +
>> +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<AclTree>| {
>> +            reduce_handle.dispatch(tree);
>> +        })));
>> +    });
>> +
>> +    let context = AclContext {
>> +        acl_tree,
>> +        _abort_guard: Rc::new(AsyncAbortGuard::spawn(
>> +            async move { LocalAclTree::load().await },
>> +        )),
>> +    };
>> +
>> +    html!(
>> +        <ContextProvider<AclContext> context={context} >
>> +            {props.children.clone()}
>> +        </ContextProvider<AclContext>>
>> +    )
>> +}
>> +
>> +#[derive(Clone, PartialEq)]
>> +pub(crate) struct LocalAclTree {
>> +    acl_tree: Rc<AclTree>,
>> +}
>> +
>> +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<SavedAclNodes> = 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<AclListItem> =
>> +            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<SavedAclNodes> = PersistentState::new(Self::LOCAL_KEY);
>> +        saved_tree.update(to_save);
>> +    }
>> +
>> +    fn get_current_authid() -> Option<Authid> {
>> +        let authid = CLIENT.with_borrow(|t| t.get_auth())?;
>> +        authid.userid.parse::<Authid>().ok()
>> +    }
>> +}
>> +
>> +impl Reducible for LocalAclTree {
>> +    type Action = Rc<AclTree>;
>> +
>> +    fn reduce(self: Rc<Self>, action: Self::Action) -> Rc<Self> {
>> +        Rc::new(Self { acl_tree: action })
>> +    }
>> +}
>> +
>> +#[derive(Deserialize, Serialize, PartialEq, Clone, Default)]
>> +struct SavedAclNodes {
>> +    authid: Option<Authid>,
>> +    nodes: Vec<AclListItem>,
>> +}
>> +
>> +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