From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Datacenter Manager development discussion
<pdm-devel@lists.proxmox.com>,
Shannon Sterz <s.sterz@proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/10] add support for checking acl permissions in (yew) front-ends
Date: Thu, 13 Nov 2025 18:06:48 +0100 [thread overview]
Message-ID: <a18c420b-d523-400d-909b-f223b6b20945@proxmox.com> (raw)
In-Reply-To: <DE7PRA0KHNYN.32IYVQTQ2Q5YD@proxmox.com>
Am 13.11.25 um 17:39 schrieb Shannon Sterz:
> On Thu Nov 13, 2025 at 5:18 PM CET, Thomas Lamprecht wrote:
>> Am 13.11.25 um 15:26 schrieb Shannon Sterz:
>>> i suppose i could also try to extract the roles with `AclTree::roles`,
>>> which extracts the roles via `AclTreeNode::extract_roles` which is
>>> already somewhat opinionated about how groups should work here. not sure
>>> what is ideal here.
>>
>> That I did not looked closely enough into to answer for sure.
>
> thanks for your quick reply, i looked into this some more after i send
> this mail and i have a version of this series here now that uses
> `AclTreeNode::extract_roles` to extract the roles for user in general.
> the upshot of this approach is that we already use the acl tree's
> implementation here instead of manually extracting the roles. so once we
> add better support for groups in the acl tree, we'll pick them up here
> for free.
>
> context: the acl tree has somewhat half-baked support for groups already
> and supports extracting them correctly for a given user. the part that
> is missing there is the look up of what groups a user belongs too. so
> relying on that seems somewhat safe to me as the pre-existing codepaths
> have been around for a while now.
>
> it looks somewhat like this:
>
> if all_for_authid {
> if let Some(auth_id) = auth_id_filter {
> for (role, propagate) in node.extract_roles(auth_id, true) {
> to_return.push(AclListItem {
> path: path_str.to_owned(),
> propagate,
> // do not disclose what groups exist and by
> // making them always look like user permissions
> ugid_type: AclUgidType::User,
> ugid: auth_id.to_string(),
> roleid: role.to_string(),
> })
> }
> }
>
> what do you think? should we go with this approach instead?
Again, only glanced at your series and this hunk, so do take this with a grain
of salt (and a potential for Fabian's rejection ;), but it does look OK to me,
and would indeed be a bit more future proof w.r.t. hedging against leaking
any groups themselves.
_______________________________________________
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-11-13 17:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-06 14:38 Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH proxmox v3 1/5] access-control: add acl feature to only expose types and the AclTree Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH proxmox v3 2/5] access-control: use format strings where possible Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH proxmox v3 3/5] access-control: move functions querying privileges to the AclTree Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH proxmox v3 4/5] access-control: derive Debug and PartialEq on AclTree and AclTreeNode Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH proxmox v3 5/5] access-control: allow reading all acls of the current authid Shannon Sterz
2025-11-13 10:23 ` Lukas Wagner
2025-11-06 14:38 ` [pdm-devel] [PATCH yew-comp v3 1/2] acl_context: add AclContext and AclContextProvider Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH yew-comp v3 2/2] http_helpers: reload LocalAclTree when logging in or refreshing a ticket Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH datacenter-manager v3 1/3] server/api-types: move AccessControlConfig to shared api types Shannon Sterz
2025-11-13 10:15 ` Lukas Wagner
2025-11-13 10:23 ` Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH datacenter-manager v3 2/3] ui: add an AclContext via the AclContextProvider to the main app ui Shannon Sterz
2025-11-06 14:38 ` [pdm-devel] [PATCH datacenter-manager v3 3/3] ui: main menu: use the AclContext to hide the Notes if appropriate Shannon Sterz
2025-11-13 10:21 ` [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/10] add support for checking acl permissions in (yew) front-ends Lukas Wagner
2025-11-13 10:26 ` Shannon Sterz
2025-11-13 13:58 ` Fabian Grünbichler
2025-11-13 14:27 ` Shannon Sterz
2025-11-13 16:18 ` Thomas Lamprecht
2025-11-13 16:39 ` Shannon Sterz
2025-11-13 17:06 ` Thomas Lamprecht [this message]
2025-11-14 14:44 ` [pdm-devel] Superseded: " 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=a18c420b-d523-400d-909b-f223b6b20945@proxmox.com \
--to=t.lamprecht@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 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.