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
next prev parent 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