mirror of
https://github.com/PegaProx/project-pegaprox.git
synced 2026-08-12 15:27:47 +08:00
dd03f7dc0e
* oidc: apply custom roles from group mappings instead of dropping them
A group mapping onto a custom (e.g. tenant-scoped) role never took effect. The
mapping loop ranked roles through a hard-coded {viewer: 0, user: 1, admin: 2}
table read with .get(role, 0), so any custom role tied with viewer and the
strict `>` comparison never fired — while the log line still reported
"Custom group mapping matched", leaving no trace of why the user ended up on
oidc_default_role. Custom roles are offered in the config UI (the Default role
dropdown lists every non-builtin role) and accepted by the settings validator,
so the intent was clearly that they work.
Rank custom roles between user and admin: an explicit mapping onto a custom
role now beats the coarse viewer/user defaults, but can never demote someone
matched by the admin group. A role that came from oidc_default_role is also
tracked separately, so a custom default no longer swallows every mapping.
Adds tests/test_oidc_group_mapping.py — the four custom-role cases fail before
this change, the four no-regression cases (admin precedence, builtin ladder,
unmatched default) pass both before and after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(oidc): don't let a matched mapping demote a configured admin default
Review feedback on #682: role_from_default let any matching mapping replace the
configured default, including when that default is admin — so an install using
oidc_default_role='admin' would be downgraded to viewer/user/a custom role on the
first matching group. The effect is a loss of admin access rather than an
escalation, but it changes long-standing behaviour and can leave an install
without access to admin-only workflows.
Keep the rule that the configured default stands for "no group matched" and may
therefore be replaced by a group that did match, but exempt admin from it. Adds
the regression test, which fails without the exemption.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(oidc): warn when several custom-role group mappings match
Review feedback on #682: custom roles all share one precedence level, so when a
user matches more than one custom-role mapping the first entry in the list wins
and the rest tie out — the outcome depends on mapping order.
Keeping first-match-wins: the mapping list is an ordered list in the UI, so
reading it top-down is the least surprising rule, and it is deterministic for a
given config. Defining a real precedence *between* custom roles would mean
ranking their permission sets against each other, which is a product decision
(permission count is a poor proxy for privilege, and tenant-scoped roles are not
comparable at all) rather than something to settle inside a bug fix.
What was worth fixing is the silence: several matches now log a warning naming
the roles, the one applied, and how to change the outcome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
PegaProx test suite
Authorization / tenant-isolation regression suite
The single biggest security risk in a multi-tenant cluster manager is broken access control (BOLA / IDOR / tenant escape) — and static scanners (Aikido, CodeAnt) can't see logic-level authorization bugs. Historically these were caught only by hand review (#490 / #493 / #495 / #555 …). This suite turns those invariants into permanent, executable guards so they can't silently regress.
Running
pip install -r requirements-dev.txt # once
python -m pytest # from the repo root
No live PVE/ESXi/XCP-ng cluster is required. user_can_access_vm,
get_user_clusters, has_permission etc. only read the DB (users, tenants,
vm_acls, pool_permissions), so each test seeds a throwaway encrypted DB in a
temp dir and asserts an access decision.
How the harness works (conftest.py)
gevent.monkey.patch_all()runs first (auth/db import gevent internals).- The
dbfixture pointspegaprox.core.db.{CONFIG_DIR,DATABASE_FILE,KEY_FILE}at a per-test temp dir, resets both DB singletons (_dbandPegaProxDB._instance), and clears rbac's process-global caches (tenants_db,_custom_roles_cache,_vm_acls_cache,_pool_membership_cache) so tests are fully order-independent. - The
seedfixture exposesseed.user(),seed.tenant(),seed.vm_acl(),seed.pool()bound to that DB.
Files
| File | Covers |
|---|---|
test_authz_isolation.py |
cross-tenant BOLA, cluster scoping, VM-ACL additive/restrictive model, the #555 pool/ACL cluster-reach guard, admin bypass, denied-perm subtraction |
test_authz_api_token.py |
API-token privilege floor (min(token, owner)), no admin-bypass for an admin-owned viewer token, mint-ceiling |
test_authz_pool.py |
pool.admin vs granular pool perms, pool grant does not leak to non-member VMs or across clusters |
Broken-access-control regression guard
test_vm_acl_inherit_role_false_restricts_to_listed_perms guards a
broken-access-control bug that has been fixed: the vm_acls table now has
an inherit_role column (added with an idempotent migration), so an ACL saved
with inherit_role=False (the UI's "custom permissions" mode) is honoured and
user_can_access_vm restricts to the listed perms instead of granting full
access. Coercion is strict on both write paths, so a stringy "false" cannot
re-broaden access. The test now passes as a plain assertion (no xfail); keep
it — it fails if the column, migration, or restrictive read is ever regressed.