public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>,
	Shannon Sterz <s.sterz@proxmox.com>
Subject: Re: [pdm-devel] [PATCH yew-comp v2 1/2] acl_context: add AclContext and AclContextProvider
Date: Thu, 23 Oct 2025 12:00:56 +0200	[thread overview]
Message-ID: <51892c50-105b-4793-ab51-f7507dd02117@proxmox.com> (raw)
In-Reply-To: <20251022131126.358790-6-s.sterz@proxmox.com>

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?

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


  reply	other threads:[~2025-10-23 10:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-22 13:11 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v2 00/10] add support for checking acl permissions in (yew) front-ends Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH proxmox v2 1/4] access-control: add acl feature to only expose types and the AclTree Shannon Sterz
2025-10-23  9:24   ` Dominik Csapak
2025-10-23 11:32     ` Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH proxmox v2 2/4] access-control: move functions querying privileges to " Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH proxmox v2 3/4] access-control: derive Debug and PartialEq on AclTree and AclTreeNode Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH proxmox v2 4/4] access-control: allow reading all acls of the current authid Shannon Sterz
2025-10-23  9:31   ` Dominik Csapak
2025-10-23 11:32     ` Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH yew-comp v2 1/2] acl_context: add AclContext and AclContextProvider Shannon Sterz
2025-10-23 10:00   ` Dominik Csapak [this message]
2025-10-23 11:33     ` Shannon Sterz
2025-10-23 11:39       ` Dominik Csapak
2025-10-22 13:11 ` [pdm-devel] [PATCH yew-comp v2 2/2] http_helpers: reload LocalAclTree when logging in or refreshing a ticket Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH datacenter-manager v2 1/2] server/api-types: move AccessControlConfig to shared api types Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH datacenter-manager v2 2/2] ui: add an AclContext via the AclContextProvider to the main app ui Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH yew-comp v2 1/1] notes view: allow hiding the toolbar if editing isn't supported Shannon Sterz
2025-10-23  9:36   ` Dominik Csapak
2025-10-23 11:33     ` Shannon Sterz
2025-10-22 13:11 ` [pdm-devel] [PATCH datacenter-manager v2 1/1] ui: main menu: use the AclContext to hide the Notes if appropriate Shannon Sterz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51892c50-105b-4793-ab51-f7507dd02117@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=s.sterz@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal