From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>
Cc: Proxmox Datacenter Manager development discussion
<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH proxmox v2 4/4] access-control: allow reading all acls of the current authid
Date: Thu, 23 Oct 2025 13:32:59 +0200 [thread overview]
Message-ID: <DDPO3DCC2KIG.3UOXZ0WJAO3K2@proxmox.com> (raw)
In-Reply-To: <6ba2fbad-9058-4263-b98d-a1255234d7e9@proxmox.com>
On Thu Oct 23, 2025 at 11:31 AM CEST, Dominik Csapak wrote:
> High level question:
>
> in proxmox-backup we already have a 'list_permissions' api call
>
> would that (or something like that) not be better suited for the purpose?
> (i don't know if it would be easy to refactor that api call here;
> probably not because it's pbs specific)
>
> since with this api call, we'd only have the 'raw' acl entries and
> must to calculate things like propagation, etc. on the frontend?
>
> Or am I wrong with that assumption?
the endpoint also exists in pdm, but there are couple of things that
prevented me from using it:
- it's not yet in a common crate making it more brittle to use in
yew-comp. though that can of course be changed easily.
- it gives you permissions, not roles. the acl tree, however, works with
roles. the idea behind this patch series was to re-use the logic of
the acl tree as much as possible. using permissions instead of roles
would mean we:
- either need to implement another datastructure on the front-end
that queries something like an acl tree, but with permissions.
duplicating quite a bit of logic and also being more error prone.
we still need to traverse the tree after all to make sure that we
have the correct path.
- adapting the acl tree to work with permissions directly as well.
this would mean quite a bit of churn in the acl tree, we need to
make sure that none of the current use sites break.
- the performance impact of doing it this way wasn't to big in my
testing. the actual acl tree in the front-end only consists of the
users roles. so it isn't too big by itself. i doubt the propagation
logic by itself costs us that much, as it often can just act as a
short-circuit (i.e. meaning we return immediately if a role
propagates).
i can however look into stress testing this a bit more to see if it
would cause issues given a very complex acl tree for one specific user.
> On 10/22/25 3:11 PM, Shannon Sterz wrote:
>> adds a parameter to the `API_METHOD_READ_ACL` endpoint to allow
>> listing all ACL entries of the currently authenticated Authid.
>> allowing a user to see their own ACLs does not really exposes any
>> additional confidential information. however, being able to query this
>> information allows us, for example, to adapt ui components to a users
>> capabilities.
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>> proxmox-access-control/src/api/acl.rs | 37 ++++++++++++++++++++++-----
>> 1 file changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/proxmox-access-control/src/api/acl.rs b/proxmox-access-control/src/api/acl.rs
>> index 0194d517..07222939 100644
>> --- a/proxmox-access-control/src/api/acl.rs
>> +++ b/proxmox-access-control/src/api/acl.rs
>> @@ -23,6 +23,12 @@ use crate::CachedUserInfo;
>> optional: true,
>> default: false,
>> },
>> + "exact-authid": {
>> + description: "Whether to return ACL entries for the exact current authid only.",
>> + type: bool,
>> + optional: true,
>> + default: false,
>> + }
>> },
>> },
>> returns: {
>> @@ -34,13 +40,17 @@ use crate::CachedUserInfo;
>> },
>> access: {
>> permission: &Permission::Anybody,
>> - description: "Returns all ACLs if user has sufficient privileges on this endpoint, otherwise it is limited to the user's API tokens.",
>> + description: "Returns all ACLs if a user has sufficient privileges on this endpoint. \
>> + Otherwise it is limited to the user's API tokens. However, if `exact-authid` is \
>> + specified, all ACLs of the current Auhtid will be returned, whether the Authid has \
>> + privileges to list other ACLs here or not.",
>> },
>> )]
>> /// Get ACL entries, can be filter by path.
>> pub fn read_acl(
>> path: Option<String>,
>> exact: bool,
>> + exact_authid: bool,
>> rpcenv: &mut dyn RpcEnvironment,
>> ) -> Result<Vec<AclListItem>, Error> {
>> let auth_id = rpcenv
>> @@ -58,7 +68,11 @@ pub fn read_acl(
>> )
>> .is_err();
>>
>> - let filter = if filter_entries { Some(auth_id) } else { None };
>> + let filter = if filter_entries || exact_authid {
>> + Some(auth_id)
>> + } else {
>> + None
>> + };
>>
>> let (mut tree, digest) = crate::acl::config()?;
>>
>> @@ -74,7 +88,13 @@ pub fn read_acl(
>>
>> rpcenv["digest"] = hex::encode(digest).into();
>>
>> - Ok(extract_acl_node_data(node, path.as_deref(), exact, &filter))
>> + Ok(extract_acl_node_data(
>> + node,
>> + path.as_deref(),
>> + exact_authid,
>> + exact,
>> + &filter,
>> + ))
>> }
>>
>> #[api(
>> @@ -241,7 +261,8 @@ pub fn update_acl(
>> fn extract_acl_node_data(
>> node: &AclTreeNode,
>> path: Option<&str>,
>> - exact: bool,
>> + exact_authid: bool,
>> + exact_path: bool,
>> auth_id_filter: &Option<Authid>,
>> ) -> Vec<AclListItem> {
>> // tokens can't have tokens, so we can early return
>> @@ -259,7 +280,11 @@ fn extract_acl_node_data(
>>
>> for (user, roles) in &node.users {
>> if let Some(auth_id_filter) = auth_id_filter {
>> - if !user.is_token() || user.user() != auth_id_filter.user() {
>> + if exact_authid {
>> + if user != auth_id_filter {
>> + continue;
>> + }
>> + } else if !user.is_token() || user.user() != auth_id_filter.user() {
>> continue;
>> }
>> }
>> @@ -291,7 +316,7 @@ fn extract_acl_node_data(
>> }
>> }
>>
>> - if !exact {
>> + if !exact_path {
>> nodes.extend(
>> node.children
>> .iter()
_______________________________________________
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 11:32 UTC|newest]
Thread overview: 22+ 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 [this message]
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
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
2025-10-24 14:53 ` [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-24 14:51 Shannon Sterz
2025-10-24 14:51 ` [pdm-devel] [PATCH proxmox v2 4/4] access-control: allow reading all acls of the current authid 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=DDPO3DCC2KIG.3UOXZ0WJAO3K2@proxmox.com \
--to=s.sterz@proxmox.com \
--cc=d.csapak@proxmox.com \
--cc=pdm-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.