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

Permissions & RBAC Fixes — Implementation Plan

Date: 2026-02-05 Status: Updated after re-audit Depends on: 2026-02-05-permissions-audit-design.md Scope: Phased fix of all permission issues identified in audit and re-audit

---

Phasing Strategy

Fixes are grouped into four phases by severity and dependency:

• Phase A: Critical security fixes (must-do before any multi-user use)
• Phase B: High-severity gaps (needed for MSP readiness)
• Phase C: Medium-severity improvements (better UX and consistency)
• Phase D: Low-severity cleanup (nice-to-haves)

Each phase is independently deployable. Phase A should be done as a single PR.

What Changed Since Initial Plan

The RBAC commit (34daa26) added infrastructure (permissions.py, usePermissions.ts, is_super_admin), but the re-audit revealed:

• permissions.py is dead code — no endpoint imports it
• require_engineer_or_admin has a bug blocking team admins with viewer role
• 49 endpoints bypass get_current_active_user
• The role field still accepts arbitrary strings
• TreeEditorPage uses wrong permission check for edit mode

These new findings are integrated into the phases below.

---

Phase A: Critical Security Fixes

Branch: fix/critical-security Priority: Immediate — blocks all other work

A1. Lock Down Registration Role Field

Files to modify:

• backend/app/schemas/user.py — Constrain role to Literal["engineer", "viewer"] or remove entirely
• backend/app/api/endpoints/auth.py — Hardcode role="engineer" in registration

Changes:

# schemas/user.py — Option A: Remove role from UserCreate entirely
class UserCreate(UserBase):
    password: str = Field(..., min_length=10)
    # role field REMOVED — always defaults to "engineer"

# schemas/user.py — Option B: Constrain with Literal (if viewer self-registration is desired)
from typing import Literal
class UserCreate(UserBase):
    role: Literal["engineer", "viewer"] = "engineer"
    password: str = Field(..., min_length=10)

# auth.py — If using Option A, hardcode:
new_user = User(
    ...
    role="engineer",  # Always engineer; changed only via admin endpoint
    ...
)

Tests to add:

• Register with role: "admin" → should still be engineer (or 422 with Literal)
• Register with role: "super_admin" → rejected
• Register with arbitrary string → rejected
• Register with default → engineer

A2. Escape HTML Export Output

Files to modify:

• backend/app/api/endpoints/sessions.py — HTML export function

Changes:

from html import escape

# Escape ALL user-provided content in _generate_html_export()
html.append(f'<title>{escape(tree_name)}</title>')
html.append(f'<h1>{escape(tree_name)}</h1>')
html.append(f'<p><strong>Ticket:</strong> {escape(session.ticket_number or "")}</p>')
html.append(f'<p><strong>Client:</strong> {escape(session.client_name or "")}</p>')
html.append(f'<div class="scratchpad">{escape(scratchpad)}</div>')
html.append(f'<h3>Step {i}: {escape(question)}</h3>')
html.append(f'<p class="answer">Answer: {escape(answer)}</p>')
html.append(f'<p class="notes">Notes: {escape(notes)}</p>')

Tests to add:

• Export session with <script>alert('xss')</script> in ticket_number → verify <script> in output
• Export session with <img onerror=...> in notes → verify escaped
• Export session with &, ", ' in fields → verify entities

A3. Fail on Default Secret Key in Production

Files to modify:

• backend/app/core/config.py

Changes:

@validator("SECRET_KEY")
def validate_secret_key(cls, v):
    default = "your-secret-key-change-in-production-use-openssl-rand-hex-32"
    if not v or v == default:
        import os
        if os.getenv("DEBUG", "true").lower() != "true":
            raise ValueError("SECRET_KEY must be set to a secure value in production")
    return v

A4. Add Role Enum Constraint to Database

Files to modify:

• backend/app/models/user.py — Add check constraint
• New migration

Changes:

from sqlalchemy import CheckConstraint

class User(Base):
    __table_args__ = (
        CheckConstraint("role IN ('engineer', 'viewer')", name="ck_user_role"),
    )

Note: Role is now only engineer or viewer. The old admin role is fully replaced by is_super_admin flag (migration 010 already converted existing admin users).

Migration: alembic revision --autogenerate -m "Add role check constraint"

---

Phase B: High-Severity Gaps

Branch: fix/high-security Priority: Before production MSP use

B1. Add Tree Access Check to start_session

Files to modify:

• backend/app/api/endpoints/sessions.py

Changes: After fetching the tree, add access check matching get_tree:

can_access = (
    tree.is_default or tree.is_public or
    tree.author_id == current_user.id or
    (tree.team_id == current_user.team_id and current_user.team_id is not None) or
    current_user.is_super_admin
)
if not can_access:
    raise HTTPException(status_code=403, detail="You don't have access to this tree")

Tests to add:

• User starts session on own tree → success
• User starts session on public tree → success
• User starts session on private team tree they don't belong to → 403
• Super admin starts session on any tree → success

B2. Wire permissions.py Into Endpoints (or Delete It)

Decision needed: Wire the centralized module into all endpoints, or delete it and keep local helpers.

Recommended: Wire it in. Replace all 6 local permission helpers with imports from permissions.py.

Files to modify:

• backend/app/api/endpoints/trees.py — Import and use can_edit_tree, can_delete_tree
• backend/app/api/endpoints/steps.py — Import and use can_edit_step, can_view_step
• backend/app/api/endpoints/tags.py — Import and use can_manage_tree_tags, can_create_tag
• backend/app/api/endpoints/categories.py — Import and use can_manage_category, can_create_category
• backend/app/api/endpoints/step_categories.py — Import and use can_manage_step_category, can_create_step_category
• backend/app/api/endpoints/folders.py — Import and use can_access_tree
• backend/app/core/permissions.py — Add any missing functions (can_view_step, can_create_tag, can_access_tree)

This also fixes:

• N1 (dead code)
• Finding #10 partial (None guards from centralized module applied everywhere)

B3. Fix require_engineer_or_admin Team Admin Bug

Files to modify:

• backend/app/api/deps.py

Changes:

async def require_engineer_or_admin(
    current_user: Annotated[User, Depends(get_current_active_user)]
) -> User:
    if current_user.is_super_admin:
        return current_user
    if current_user.is_team_admin and current_user.team_id is not None:
        return current_user
    if current_user.role not in ("engineer",):
        raise HTTPException(status_code=403, detail="Requires engineer or higher role")
    return current_user

B4. Add is_active Field and Update All Dependencies

Files to modify:

• backend/app/models/user.py — Add is_active = Column(Boolean, default=True, server_default="true")
• backend/app/api/deps.py — Implement get_current_active_user check
• All endpoint files — Change Depends(get_current_user) to Depends(get_current_active_user) (49 usages)
• New migration

Changes:

# deps.py
async def get_current_active_user(
    current_user: Annotated[User, Depends(get_current_user)]
) -> User:
    if not current_user.is_active:
        raise HTTPException(status_code=403, detail="Account has been deactivated")
    return current_user

B5. Create Admin User Management Endpoints

New file: backend/app/api/endpoints/admin.py

Endpoints:

Method	Path	Description
GET	/api/v1/admin/users	List all users (super admin only)
GET	/api/v1/admin/users/{id}	Get user details (super admin only)
PUT	/api/v1/admin/users/{id}/role	Change user role (super admin only)
PUT	/api/v1/admin/users/{id}/team-admin	Toggle is_team_admin (super admin only)
PUT	/api/v1/admin/users/{id}/deactivate	Deactivate account (super admin only)
PUT	/api/v1/admin/users/{id}/activate	Reactivate account (super admin only)

Files to modify:

• backend/app/api/router.py — Register admin router
• backend/app/schemas/user.py — Add admin response/update schemas

B6. Add Rate Limiting

New dependency: slowapi

Files to modify:

• backend/requirements.txt — Add slowapi
• backend/app/main.py — Add rate limiter middleware
• backend/app/api/endpoints/auth.py — Apply rate limits

Rate limits:

Endpoint	Limit
POST /auth/login	5/minute per IP
POST /auth/register	3/minute per IP
POST /auth/refresh	10/minute per IP
GET /invites/validate/{code}	5/minute per IP

B7. Token Revocation (Short-Term Fix)

Approach: Store refresh tokens in DB so they can be revoked. Reduce access token TTL.

Files to modify:

• backend/app/models/ — New RefreshToken model (token hash, user_id, expires_at, revoked_at)
• backend/app/api/endpoints/auth.py — Store on login, validate on refresh, revoke on logout
• backend/app/core/config.py — Reduce ACCESS_TOKEN_EXPIRE_MINUTES to 5
• New migration

Logout becomes meaningful:

@router.post("/logout")
async def logout(payload = Depends(get_refresh_token_payload), db = Depends(get_db)):
    await db.execute(
        update(RefreshToken)
        .where(RefreshToken.token_hash == hash_token(payload["jti"]))
        .values(revoked_at=datetime.now(timezone.utc))
    )
    return {"message": "Successfully logged out"}

---

Phase C: Medium-Severity Improvements

Branch: feat/permissions-ux Priority: Better UX and consistency

C1. Frontend: Add 403 Handling and Route Guards

Files to modify:

• frontend/src/api/client.ts — Add 403 interceptor
• frontend/src/components/layout/ProtectedRoute.tsx — Add requiredRole prop
• frontend/src/router.tsx — Apply role guards to admin routes

403 interceptor:

if (error.response?.status === 403) {
  toast.error("You don't have permission to perform this action")
  return Promise.reject(error)
}

ProtectedRoute enhancement:

interface ProtectedRouteProps {
  requiredRole?: UserRole | UserRole[]
  requireTeamAdmin?: boolean
  children: React.ReactNode
}

C2. Frontend: Fix TreeEditorPage Permission Check

Files to modify:

• frontend/src/pages/TreeEditorPage.tsx

Change: After loading the tree in edit mode, check canEditTree(tree) instead of just canCreateTrees:

useEffect(() => {
  if (tree && id && !canEditTree({ author_id: tree.author_id, team_id: tree.team_id })) {
    navigate('/trees')
  }
}, [tree, id])

C3. Frontend: Add Delete UI for Trees and Steps

Files to modify:

• frontend/src/pages/TreeLibraryPage.tsx — Add delete button gated by canDeleteTree
• frontend/src/components/step-library/StepCard.tsx — Add delete button gated by canEditStep
• frontend/src/components/common/ConfirmDialog.tsx — New reusable confirmation modal

C4. Frontend: Wire usePermissions Into All Relevant Components

Files to modify:

• frontend/src/components/step-library/StepLibraryBrowser.tsx — Use canCreateSteps for create button
• frontend/src/components/step-library/CustomStepModal.tsx — Check canCreateSteps
• frontend/src/components/tree-editor/NodeList.tsx — Add confirmation dialog for node deletion

C5. Backend: Fix None == None in All Local Helpers

Files to modify (if not already fixed by B2):

• backend/app/api/endpoints/steps.py — can_view_step() null guard
• backend/app/api/endpoints/categories.py — can_manage_category() null guard
• backend/app/api/endpoints/step_categories.py — can_manage_step_category() null guard

C6. Backend: Fix Admin Tree List Inconsistency

Files to modify:

• backend/app/api/endpoints/trees.py

Change: Add super admin bypass to build_tree_access_filter:

def build_tree_access_filter(current_user: User):
    conditions = [
        Tree.is_default == True,
        Tree.is_public == True,
        Tree.author_id == current_user.id,
    ]
    if current_user.team_id:
        conditions.append(Tree.team_id == current_user.team_id)
    if current_user.is_super_admin:
        conditions.append(sqlalchemy.true())
    return or_(*conditions)

C7. Backend: Add Audit Log Table

New file: backend/app/models/audit_log.py

class AuditLog(Base):
    __tablename__ = "audit_logs"
    id = Column(UUID, primary_key=True, default=uuid4)
    user_id = Column(UUID, ForeignKey("users.id"))
    action = Column(String(50))       # "tree.delete", "user.role_change", etc.
    resource_type = Column(String(50)) # "tree", "step", "user", etc.
    resource_id = Column(UUID)
    details = Column(JSONB)            # Before/after values
    created_at = Column(DateTime(timezone=True), default=lambda: datetime.now(timezone.utc))

Integrate into admin endpoints and destructive actions.

---

Phase D: Low-Severity Cleanup

Branch: chore/permissions-cleanup Priority: When convenient

D1. Define and Enforce Viewer Role Permissions

Decide what viewers can/cannot do and enforce consistently:

• Proposed: Viewers can browse trees, start sessions, view steps, and rate steps. Cannot create trees, steps, folders, or manage tags/categories.
• Add require_engineer_or_admin to folder creation and tag creation endpoints.

D2. Add Password Complexity Validation

Files to modify: backend/app/schemas/user.py

Add regex validator: at least one uppercase, one lowercase, one digit, 10+ characters.

D3. Soft Delete Cascade Cleanup

When soft-deleting a tree, also:

• Remove from all folder assignments
• Remove tag assignments
• Keep sessions intact (they have the snapshot)

D4. Remove Debug Endpoint in Production

Files to modify: backend/app/main.py

Conditionally register /debug/cors only when DEBUG=true.

D5. Escape Wildcards in Tag Search

Files to modify: backend/app/api/endpoints/tags.py

q_escaped = q.replace("%", "\\%").replace("_", "\\_")
query = select(TreeTag).where(TreeTag.name.ilike(f"%{q_escaped}%"))

---

Execution Order Summary

Phase A (Critical) ──── Single PR, immediate
  A1. Lock down registration role field
  A2. Escape HTML export
  A3. Fail on default secret key
  A4. Add role DB constraint

Phase B (High) ──── 3-4 PRs
  B1. Tree access check on start_session
  B2. Wire permissions.py into endpoints (fixes dead code + None guards)
  B3. Fix require_engineer_or_admin team admin bug
  B4. is_active field + update all 49 endpoint dependencies
  B5. Admin user management endpoints
  B6. Rate limiting
  B7. Token revocation

Phase C (Medium) ──── 3-4 PRs
  C1. Frontend 403 handling + route guards
  C2. Fix TreeEditorPage canEditTree check
  C3. Delete UI for trees and steps
  C4. Wire usePermissions into all components
  C5. Fix None==None in all local helpers (if not done in B2)
  C6. Fix admin tree list inconsistency
  C7. Audit log table

Phase D (Low) ──── As convenient
  D1. Viewer role enforcement
  D2. Password complexity
  D3. Soft delete cascade cleanup
  D4. Remove debug endpoint in prod
  D5. Escape wildcards in tag search

---

Estimated Scope

Phase	Backend Changes	Frontend Changes	Migrations	New Tests
A	4 files modified	0	1	~8 tests
B	10+ files modified, 2 new	0	2	~20 tests
C	3 files modified, 2 new	6 files modified, 1 new	1	~10 tests
D	4 files modified	0	0	~5 tests

---

Testing Strategy

Each phase should include:

1. Unit tests for new/changed permission checks
2. Integration tests for cross-user access scenarios
3. Negative tests — verify unauthorized access returns 403, not 500
4. Manual verification of frontend behavior per role

Priority test scenarios:

• Register with role: "admin" → rejected or ignored
• Register with arbitrary role string → rejected
• User A accesses User B's session → 403
• Viewer creates a tree → 403
• Team admin with viewer role creates a tree → success (B3 fix)
• Start session on private tree without access → 403
• Login with deactivated account → 403
• Exported HTML with <script> tag → escaped
• Brute force login → rate limited after 5 attempts
• permissions.py functions match endpoint behavior (after B2)

---

Notes

• Phase A changes are backward-compatible — no frontend changes needed
• Phase B is larger than originally planned due to dead code wiring (B2) and dependency chain fix (B4)
• B2 (wiring permissions.py) and B4 (is_active dependency chain) touch many files — plan for careful review
• Token revocation (B7) is the most complex change — consider deferring to a dedicated PR
• Phase C partially started: usePermissions hook exists, TreeLibraryPage and TreeEditorPage use it
• All require_admin checks should use is_super_admin (never role == "admin")