Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/10] add support for checking acl permissions in (yew) front-ends

From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: "Proxmox Datacenter Manager development discussion"
	<pdm-devel@lists.proxmox.com>,
	"Shannon Sterz" <s.sterz@proxmox.com>,
	"Fabian Grünbichler" <f.gruenbichler@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 17:18:14 +0100	[thread overview]
Message-ID: <4793f1cc-d93a-46f6-b129-f95ee2b3b1ac@proxmox.com> (raw)
In-Reply-To: <DE7MY3S088X9.1DRNLWXDND6BX@proxmox.com>

Am 13.11.25 um 15:26 schrieb Shannon Sterz:
>> high-level question:
>>
>> the actual privilege checks in the backend use the full set of ACLs. the
>> frontend can only ever see a subset of ACLs, since giving it all ACLs
>> would leak a lot of sensitive information.
>>
>> doesn't that mean that the frontend will make wrong decisions in some
>> scenarios?
>>
>> e.g., the backend currently doesn't return any group ACLs if you do
>> exact filtering. but group ACLs can influence the ACL resolution
> 
> as discussed off list, that'd be true if the filter in
> `extract_acl_node_data` isn't adapted. i'll add a fixme comment to the
> next version of this series for now.
> 
> how exactely to takle this will depend on how we implement groups:
> 
> * does a user know that they are part of a group?

No general objection, a lot of ACL system work that way, but should not
be required.

> * if disclosing such membership is fine, is it fine to disclose what the
>   group has access to in all cases? e.g. what if the user is part of a
>   group, but certain acl entries are then restricted on top via a
>   NoAccess privilege or similar?

Disclosing access is IMO always fine, that can basically be probed anyway.
Where that access comes from does not really matter here.

> * will looking up whether the user is part of a group be handled by the
>   acl tree directly? (this is at least indicated by comments already
>   present in `AclTreeNode::extract_group_roles()`)

Probably, otherwise it would need to get the relevant acl's "injected".

> most of these are difficult to answer without actually tackling an
> implementation of the group feature. not entirely sure how i can address
> this here beside adding that `fixme` comment.

Seems OK to me. If we need more changes to make this safe enough for
groups we can still change this, be it by adding a new endpoint and
deprecating the old one, where the old one would then probably ignores
groups for the rest of its existence.
One option could then be to have something like the heuristic in PVE
but more generic (and maybe we're able to make it a bit easier to
understand), but tbh. that isn't exactly perfect either, and so trying
this route until we get an actual blocker seems still worthwhile to me.

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

_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel