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

[Dominik Csapak <d.csapak@proxmox.com>]

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 <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