fix(rbac): resolve lock-order deadlocks between sessions, users, groups and roles

[VibeSWE]

Description

Fixes deadlocks between the process-wide RBAC maps in src/rbac/map.rs (SESSIONS, USERS, USER_GROUPS, ROLES). These were found when the PR #29 test run hung. parking_lot::RwLock is task-fair: a queued writer blocks new readers, including a second read by a thread that already holds one. Five cycles, each reproduced by a test that timed out before the fix:

1. First Basic login reads USERS twice. Users::authorize held its USERS read guard, took SESSIONS (write), then read USERS again in check_auth. Any user write queued in between (an OIDC login or refresh, API key creation) hung the login.
2. The reported three-way cycle: login (USERS → SESSIONS), a session check (SESSIONS → USERS) and a queued user write.
3. add_roles / remove_roles / change_password_hash took USERS (write) → SESSIONS (write), the reverse of check_auth. This needs only an admin change and any request with a session.
4. Querier delete_user held USER_GROUPS (write) while refreshing sessions, the reverse of check_auth.
5. GET /users and get_user_internal (existed before this PR, found in review) render users while holding USERS, and to_prism_user read USERS again.

Fix: one lock order, documented in rbac/map.rs: SESSIONS → USERS → USER_GROUPS → ROLES → DEFAULT_ROLE, with no second guard on a map a thread already holds.

• authorize checks the password with no lock held, then takes SESSIONS (write), re-reads the user and creates the session only if the stored password hash is unchanged (otherwise it checks again, up to 3 times). A password change, role change or deletion during a login therefore can't leave a stale session, the same guarantee the old nesting gave by accident. Permissions come from the re-read user.
• add_roles, remove_roles, change_password_hash and put_user take SESSIONS before USERS. This also closes an existing gap where a login could create a session from the user put_user was replacing.
• Querier group removal updates a copy of each group with no guard held, then writes it back.
• to_prism_user reads roles and groups from the user record it is given; both callers already pass the map entry, so output is identical.

Review follow-ups (in the same PR):

• /login stale session: the cookie exchange built its 1-hour session from an unlocked copy of the user, so a password reset, role removal or deletion during login could leave a stale cookie. authorize and exchange_basic_for_cookie now share Users::new_basic_session, which re-reads the user under SESSIONS and checks the hash before tracking the session.
• Basic tenant lookups (get_user_tenant_from_basic, validate_basic_user_tenant_id, mutate_request_with_basic_user) copy candidates out of USERS and run Argon2 with no lock held.
• Querier delete_user updates in-memory groups only after put_metadata succeeds, removing just that member.

No other behavior changes.

Tests: rbac::lock_order_tests (20 tests). Each test forces the interleaving at lock boundaries, detects a queued writer with RwLock::is_locked_exclusive(), and waits at most 20 s. On a detected deadlock it aborts the test process, because the stuck threads would otherwise hang every later test. Coverage: each cycle above, first Basic login, sessions using group permissions, a password change, role change, same-password reset or deletion during a first login, and a stress test running logins, session checks and user writes concurrently. Before the fix, that stress test deadlocked in 3 of 3 runs.

Validation: cargo test: 640 passed, 0 failed. The lock-order module passed in each of 20 consecutive runs. Each new regression test fails when its fix is reverted. cargo fmt --check is clean. cargo clippy --all-targets -- -D warnings fails only on existing lints in files this PR doesn't touch (known_schema.rs, staging/reader.rs, streams.rs, handlers/http/mod.rs). Reviewed by me and two GPT-6.1-Sol reviewers, and their findings are addressed: the GET /users deadlock, aborting on a detected deadlock, and the coverage gaps.

Merge note for PR #29: a trial merge with feat/api-key-expiration has no conflicts, but its 4 api_key_auth_tests tests failed in 4 of 10 runs. Its init_auth_maps creates the api-key-expiry-admin role only inside the ROLES.get_or_init closure, which doesn't run if one of these tests initialized ROLES first. Whichever PR merges second should insert that role with mut_roles() after get_or_init. Ideally both modules would share one test setup helper and one lock.

Not covered: running out of retries in authorize (three hash changes during one login) has no deterministic test. The order is a convention; debug-build lock-order assertions would be a good follow-up.

Write-up: Mayfly wiki, "RBAC lock-order deadlock" under the Team page.

---

This PR has:

• been tested to ensure log ingestion and log query works. (Unit tests only; not exercised against a running server.)
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added documentation for new or modified features or behaviors. (No user-facing behavior change.)

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[comment-clanker]

Review: RBAC lock-order fix

The fix holds up. Four independent reviewers (one GPT-6.1-Sol, three Opus 5.5) found no remaining lock-order inversion and no double acquisition of SESSIONS, USERS, USER_GROUPS, ROLES or DEFAULT_ROLE anywhere in the crate, including callers outside the diff. The authorize re-check is correct: guards are released before check_auth and on continue, and running out of retries returns UnAuthorized. to_prism_user output is unchanged, because both callers pass the map entry itself.

Tests: 12 of the 16 lock_order_tests abort on base bd882956 and all 16 pass at head. That held over 30 runs, 5 more under 2× CPU oversubscription (worst case about 1.2 s for the module against the 20 s deadline), and 3 full lib runs (635 tests). Removing the password_hash == verified_hash re-check makes password_change_during_first_login_leaves_no_stale_session fail, so that test catches the race. No other test module touches the global RBAC maps.

One medium-severity issue and four low-severity ones, each in its own thread below. The main one is that /login still has the stale-session race this PR closes in authorize.