resolutionflow/docs/archive/2026-02-05-permissions-audit-design.md

Permissions & RBAC Audit — Design Document

Date: 2026-02-05 Status: Updated after re-audit Scope: Full-stack permissions audit across trees, steps, sessions, teams, and admin features

---

Background

ResolutionFlow has a role-based access control system with a role hierarchy (super_admin > team_admin > engineer > viewer), backed by is_super_admin and is_team_admin boolean flags on the User model, plus team-scoped resources.

Two rounds of audit were performed:

1. Initial audit — three independent reviews (UX, architecture, adversarial)
2. Re-audit — after RBAC implementation commit (34daa26) added is_super_admin, permissions.py, usePermissions.ts, and updated endpoint permission checks

Audit methodology: Each round used three specialized reviewers:

• Frontend UX audit — every page, component, API client, store, and router
• Backend architecture audit — every endpoint, dependency, model, schema, and test
• Gap analysis / devil's advocate — security gaps, edge cases, challenged assumptions

---

Current Architecture (Post-RBAC Commit)

What Was Added in Commit 34daa26

• is_super_admin boolean on User model (migration 010)
• backend/app/core/permissions.py — centralized permission functions (role hierarchy, content guards)
• frontend/src/hooks/usePermissions.ts — permission hook with role hierarchy
• require_admin dependency now checks is_super_admin instead of role == "admin"
• require_engineer_or_admin dependency blocks viewers from tree/step creation
• usePermissions hook used in TreeLibraryPage (edit/create gating) and TreeEditorPage (access guard)
• Role badge display in AppLayout header
• TreeListItem type includes team_id for frontend permission checks

What Works

• Session ownership is properly enforced — users can only see/modify their own sessions
• Folder ownership is properly scoped to the creating user
• Tree CRUD has backend permission checks (engineer+ to create, author/admin/team-admin to edit, super-admin-only delete)
• Step Library enforces visibility levels (private/team/public) and ownership for edit/delete
• Category/Tag management is properly gated to super admin and team admin roles
• Invite code management is super-admin-only
• Frontend gating (partial): Create Tree button hidden from viewers, Edit button checks ownership/team/admin
• is_super_admin flag prevents the old admin role escalation (flag has server_default=false)

What's Still Broken

The permission system still has critical vulnerabilities, a dead-code centralized module, and significant gaps.

---

Findings — Combined Status Table

Legend

• FIXED — Issue fully resolved
• PARTIAL — Infrastructure exists but incomplete or not fully wired
• OPEN — No change from original audit

#	Severity	Finding	Status	Evidence
1	CRITICAL	Self-assignable role at registration	OPEN	UserCreate still has role field (schemas/user.py:14). auth.py:78 passes user_data.role to User. No enum validation — arbitrary strings accepted. The is_super_admin escalation is blocked (server default false), but role field is unvalidated.
2	CRITICAL	XSS in HTML export	OPEN	sessions.py:349-405 — raw f-string interpolation without html.escape(). All user content (tree_name, ticket_number, client_name, question, answer, notes, scratchpad) injected unescaped.
3	CRITICAL	Default secret key in source code	OPEN	config.py:29 — default string with no production validation.
4	HIGH	No access control on start_session	OPEN	sessions.py:70-109 — checks tree existence and active status only. No tree access check. Any authenticated user can read any tree's full structure via session snapshot.
5	HIGH	No token revocation	OPEN	Logout (auth.py:188-193) is still a no-op. JWTs valid until expiry.
6	HIGH	No account deactivation (is_active)	OPEN	No is_active field. get_current_active_user (deps.py:66-72) is a no-op.
7	HIGH	No rate limiting	OPEN	No rate limiting on any endpoint.
8	HIGH	Admin flags cannot be set via API	PARTIAL	is_super_admin field added with migration 010. But no API endpoint exists to set is_super_admin or is_team_admin. Must be set via direct DB access.
9	MEDIUM	Frontend role awareness	PARTIAL	usePermissions hook exists, used in 3 files (TreeLibraryPage, TreeEditorPage, AppLayout). But: ProtectedRoute still auth-only, no route-level role guards, no 403 handling in Axios, no admin pages.
10	MEDIUM	None == None team visibility bug	PARTIAL	Fixed in trees.py:33 (if current_user.team_id else False). Still present in local helpers: steps.py:36, categories.py:31, step_categories.py:28.
11	MEDIUM	Admin list vs get tree inconsistency	OPEN	build_tree_access_filter has no admin bypass. get_tree checks is_super_admin.
12	MEDIUM	No audit logging	OPEN	No change.
13	MEDIUM	No delete UI for trees or steps	OPEN	API methods exist, no buttons wired. canDeleteTree exists in hook but unused.
14	MEDIUM	Node deletion without confirmation	OPEN	No change.
15	LOW	Viewer role barely enforced	PARTIAL	require_engineer_or_admin blocks tree/step creation. Viewers can still create folders, start sessions, etc.
16	LOW	No password complexity	OPEN	No change.
17	LOW	Soft delete cascade cleanup	OPEN	No change.
18	LOW	Debug endpoint in production	OPEN	No change.
19	LOW	Tag search wildcard escaping	OPEN	No change.
20	LOW	No team existence validation	OPEN	No change.

Summary: 0 FIXED / 4 PARTIAL / 16 OPEN

---

New Issues Found in Re-Audit

N1. permissions.py is Dead Code (HIGH)

Location: backend/app/core/permissions.py

The centralized permissions module defines can_edit_tree, can_delete_tree, can_edit_step, can_manage_category, can_manage_tree_tags, has_minimum_role, get_effective_role, and can_create_content.

None of these functions are imported or called by any endpoint. Zero imports across the entire backend.

Each endpoint file (trees.py, steps.py, categories.py, tags.py, step_categories.py, folders.py) defines its own inline permission helpers that are similar but not identical. Key differences:

Function in permissions.py	Local equivalent	Difference
can_edit_tree	trees.py:403-407 inline	Local version doesn't call can_create_content
can_manage_category	categories.py:25-33 inline	Local version lacks team_id is not None guard
can_manage_step_category	step_categories.py:22-41 inline	Local version lacks team_id is not None guard
has_minimum_role / get_effective_role	Not used anywhere	Entirely dead code

Risk: False confidence that permissions are centralized. Maintaining two diverging copies.

N2. require_engineer_or_admin Blocks Team Admins with Viewer Role (BUG)

Location: backend/app/api/deps.py:87-98

async def require_engineer_or_admin(current_user):
    if current_user.is_super_admin:
        return current_user
    if current_user.role not in ("engineer",):
        raise HTTPException(403)

Checks is_super_admin but NOT is_team_admin. A user with role="viewer" + is_team_admin=True would be blocked from creating trees — contradicting the role hierarchy where team_admin > engineer.

N3. 49 Endpoints Bypass get_current_active_user (HIGH)

Location: All endpoint files

Most endpoints depend directly on get_current_user, bypassing get_current_active_user. If is_active were implemented, deactivated users could still: list/view sessions, export, update scratchpads, manage folders, list/view categories, tags, steps, rate steps, etc.

N4. role Field Accepts Arbitrary Strings (MEDIUM)

Location: backend/app/schemas/user.py:14, backend/app/models/user.py:27

The role field is String(50) with no DB constraint or schema validation. Users could register with role="superadmin", role="root", etc. While these don't grant permissions (only is_super_admin flag does), it creates inconsistent data. The UserCreate schema describes "engineer or viewer" but doesn't enforce it.

N5. TreeEditorPage Checks canCreateTrees Not canEditTree (MEDIUM)

Location: frontend/src/pages/TreeEditorPage.tsx:81-85

The editor page uses canCreateTrees (a blanket role check) instead of canEditTree(tree) (ownership/team check). A non-author engineer can navigate to /trees/{someone-else's-id}/edit and the frontend won't redirect them — the backend will block the save, but the UX is poor (loads editor, then fails on save).

---

MSP-Specific Concerns

1. Weak multi-tenancy: Super admin transcends all teams. Team A's super admin could access Team B's client troubleshooting data.
2. No data retention/purge: Session scratchpads and notes contain sensitive client information with no cleanup mechanism.
3. No team-level session visibility: Supervisors can't review their team's sessions.
4. Step Library IP leakage: Public steps expose team-specific troubleshooting procedures to all teams.

---

Challenged Assumptions

Assumption	Reality
"Invite codes protect registration"	They don't prevent arbitrary role strings — the role field is unvalidated
"Team scoping = tenant isolation"	Super admin transcends all teams with no boundary
"permissions.py centralizes permissions"	It's dead code — no endpoint imports it
"JWT statelessness is acceptable"	For an MSP tool with client data, inability to immediately revoke access is a liability
"usePermissions hook covers the frontend"	Only used in 3 of ~10 components that need it

---

Test Coverage Gaps

The existing test suite does NOT cover:

• Cross-user resource access (user A accessing user B's sessions/folders/steps)
• Non-admin attempting tree deletion
• Viewer role restrictions
• Arbitrary role string at registration
• Team-based access controls (no team fixtures exist)
• Step library permission checks (no step tests at all)
• Category/tag/folder cross-user access
• start_session tree access bypass
• Concurrent session access across users
• Deactivated user access (blocked by no is_active field)

---

References

• Centralized permissions (dead code): backend/app/core/permissions.py
• Frontend permissions hook: frontend/src/hooks/usePermissions.ts
• Auth dependencies: backend/app/api/deps.py
• RBAC questions doc: docs/RBAC-IMPLEMENTATION-QUESTIONS.md
• Backend endpoints: backend/app/api/endpoints/
• Models: backend/app/models/
• Tests: backend/tests/