From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox Datacenter Manager development discussion
<pdm-devel@lists.proxmox.com>,
Dietmar Maurer <dietmar@proxmox.com>,
Shannon Sterz <s.sterz@proxmox.com>
Subject: Re: [pdm-devel] [PATCH proxmox 2/4] access-control: add acl api feature
Date: Wed, 9 Apr 2025 13:39:28 +0200 [thread overview]
Message-ID: <3293442a-0aed-4ab6-a6ee-5a0f8ea6b1e6@proxmox.com> (raw)
In-Reply-To: <2022147362.1680.1744196508347@webmail.proxmox.com>
On 4/9/25 13:01, Dietmar Maurer wrote:
>
>> +/// Get ACL entries, can be filter by path.
>> +pub fn read_acl(
>> + path: Option<String>,
>> + exact: bool,
>> + rpcenv: &mut dyn RpcEnvironment,
>> +) -> Result<Vec<AclListItem>, Error> {
>> + let auth_id = rpcenv
>> + .get_auth_id()
>> + .ok_or_else(|| format_err!("endpoint called without an auth id"))?
>> + .parse()?;
>> +
>> + let top_level_privs = CachedUserInfo::new()?.lookup_privs(&auth_id, &["access", "acl"]);
>> +
>> + let filter = if top_level_privs & access_conf().acl_audit_privileges() == 0 {
>> + Some(auth_id)
>> + } else {
>> + None
>> + };
>
> As discussed offline, maybe we can use CachedUserInfo::check_privs here?
>
>
maybe something like this for the update case (untested, please verify before using this!):
(the diff is for pbs, where the code was copied from)
this also includes a reformatted check for the token,non-token, same user checks
that are IMHO more readable than what we currently have
with the match, i think it's much more obvious that all cases are handled
---
let user_info = CachedUserInfo::new()?;
- let top_level_privs = user_info.lookup_privs(¤t_auth_id, &["access", "acl"]);
- if top_level_privs & PRIV_PERMISSIONS_MODIFY == 0 {
+ let has_modify_permission = user_info
+ .check_privs(
+ ¤t_auth_id,
+ &["access", "acl"],
+ PRIV_PERMISSIONS_MODIFY,
+ false,
+ )
+ .is_ok();
+
+ if !has_modify_permission {
if group.is_some() {
bail!("Unprivileged users are not allowed to create group ACL item.");
}
match &auth_id {
Some(auth_id) => {
- if current_auth_id.is_token() {
- bail!("Unprivileged API tokens can't set ACL items.");
- } else if !auth_id.is_token() {
- bail!("Unprivileged users can only set ACL items for API tokens.");
- } else if auth_id.user() != current_auth_id.user() {
- bail!("Unprivileged users can only set ACL items for their own API tokens.");
+ let same_user = auth_id.user() == current_auth_id.user();
+ match (current_auth_id.is_token(), auth_id.is_token(), same_user) {
+ (true, _, _) => bail!("Unprivileged API tokens can't set ACL items."),
+ (false, false, _) => {
+ bail!("Unprivileged users can only set ACL items for API tokens.")
+ }
+ (false, true, true) => {
+ // users are always allowed to modify ACLs for their own tokens
+ }
+ (false, true, false) => {
+ bail!("Unprivileged users can only set ACL items for their own API tokens.")
+ }
}
}
None => {
---
_______________________________________________
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-04-09 11:39 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-03 14:17 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp 0/9] ACL edit api and ui components Shannon Sterz
2025-04-03 14:17 ` [pdm-devel] [PATCH proxmox 1/4] access-control: add more types to prepare for api feature Shannon Sterz
2025-04-03 14:17 ` [pdm-devel] [PATCH proxmox 2/4] access-control: add acl " Shannon Sterz
2025-04-09 11:01 ` Dietmar Maurer
2025-04-09 11:39 ` Dominik Csapak [this message]
2025-04-09 12:58 ` Shannon Sterz
2025-04-10 6:28 ` Dominik Csapak
2025-04-10 8:17 ` Shannon Sterz
2025-04-10 10:09 ` Dominik Csapak
2025-04-11 10:29 ` Shannon Sterz
2025-04-11 10:53 ` Dominik Csapak
2025-04-11 11:40 ` Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH proxmox 3/4] access-control: add comments to roles function of AccessControlConfig Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH proxmox 4/4] access-control: add generic roles endpoint to `api` feature Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH yew-comp 1/3] api-types/role_selector: depend on common `RoleInfo` type Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH yew-comp 2/3] acl: add a view and semi-generic `EditWindow` for acl entries Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH yew-comp 3/3] role_selector/acl_edit: make api endpoint and default role configurable Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH datacenter-manager 1/2] server: use proxmox-access-control api implementations Shannon Sterz
2025-04-03 14:18 ` [pdm-devel] [PATCH datacenter-manager 2/2] ui: configuration: add panel for viewing and editing acl entries Shannon Sterz
2025-04-11 13:45 ` [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp 0/9] ACL edit api and ui components 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=3293442a-0aed-4ab6-a6ee-5a0f8ea6b1e6@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=dietmar@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