the token/user priv intersection could only honored user privs that had
the propagation flag set, reducing the scope of the token more than
intended.
the pre-existing test case actually triggered the broken behaviour, but
the expected value matched it so it was not noticed.
Fixes: e8a0cee47b "rpcenv: improve user/token intersection"
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
when multiple roles are defined on a path that share a privilege, this
randomly took the propagation flag for the priv from the last role
encountered. since perl hashes are iterated randomly, this means the
propagation flag was sometimes set correctly, and sometimes not.
note that this propagation flag is only used for display/dumping
purposes, and for intersection with token privs (see next commit).
actual handling of propagation happens on the role level in
PVE::AccessControl::roles().
modified test case (spuriously) fails without the fix.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
and check both existing groups and the groups parameter in the update
case. the following user.cfg settings can be used for testing:
user:test@pve:1:0:t:::❌
user:other@pve:1:0:t:::❌
group:test:test@pve::
group:test3:::
role:RealmUserAllocator:Realm.AllocateUser:
role:UserModifier:User.Modify:
acl:0:/access/groups/test:test@pve:UserModifier:
acl:0:/access/groups/test3:@test:UserModifier:
acl:0:/access/realm/pve:test@pve:RealmUserAllocator:
unchanged: the user 'test@pve' can allocate new '@pve' users, but
only if the created user will belong to at least one of 'test'
(direct ACL for that user) or 'test3' (indirect ACL via 'test' group)
groups.
changed: if the user 'test@pve' updates an existing user, they need
to (A) have 'User.Modify' on at least one existing group of that
user, and (B) 'User.Modify' on all of the groups passed in via the
'groups' parameter. A is the general rule for 'allowed to modify
user' across the board, but was missing for this specific variant of
the check. B was the case before, but just checking this without also
checking A allows a user to pull off-limits users into groups that
they can modify, which then in turn allows them to modify those users
via A which is now passing.
for example, without this patch 'test@pve' would be able to add
'other@pve' to either 'test' or 'test3', and then in turn call any of
the API endpoints that require 'User.Modify' on a user's group
(change TFA, change password or delete user if realm is pve, ..).
all the other userid-group checks without group_param set remain
unchanged as well, since $check_existing_user is true in that case.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
this shouldn't happen anymore, but a safeguard in case the parser ever
has a bug does not hurt.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
these are just cosmetic fixes/safeguards against future bugs -
compute_api_permissions is used to set the 'cap' object to hide parts of
the GUI that are not usable without the corresponding privs in the
backend anyway, and get_effective_permissions is only used to return the
permission tree without a specific path query.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
this could return undef for the propagation flag instead of 1/0, leading
to confusing displays of permission trees. all the actual checks using
the returned hash check for definedness anyway, so the actual
privileges checked and the displayed ones were not identical.
fixes: 7e8bcaa754
"roles()/permissions(): also return propagate flag"
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
