fix: enforce run ownership on all mutation endpoints
Add require_run_owner helper in auth.py that enforces ownership on mutation endpoints. Unowned (legacy) runs are now read-only. Applied ownership checks to: - All 4 encounter mutation endpoints - Both boss result mutation endpoints - Run update/delete endpoints - All 5 genlocke mutation endpoints (via first leg's run owner) Also sets owner_id on run creation in genlockes.py (create_genlocke, advance_leg) and adds 22 comprehensive ownership enforcement tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,84 @@
|
||||
---
|
||||
# nuzlocke-tracker-73ba
|
||||
title: Enforce run ownership on all mutation endpoints
|
||||
status: completed
|
||||
type: bug
|
||||
priority: critical
|
||||
created_at: 2026-03-21T12:18:27Z
|
||||
updated_at: 2026-03-21T12:28:35Z
|
||||
parent: nuzlocke-tracker-wwnu
|
||||
---
|
||||
|
||||
## Problem
|
||||
|
||||
Backend mutation endpoints for encounters, bosses, and run updates use `require_auth` but do NOT verify the authenticated user is the run's owner. Any authenticated user can modify any run's encounters, mark bosses as defeated, or change run settings.
|
||||
|
||||
Additionally, `_check_run_access` in `runs.py:184` allows anyone to edit unowned (legacy) runs when `require_owner=False`.
|
||||
|
||||
### Affected endpoints
|
||||
|
||||
**encounters.py** — all mutations use `require_auth` with no ownership check:
|
||||
- `POST /runs/{run_id}/encounters` (line 35)
|
||||
- `PATCH /runs/{run_id}/encounters/{encounter_id}` (line 142)
|
||||
- `DELETE /runs/{run_id}/encounters/{encounter_id}` (line 171)
|
||||
- `POST /runs/{run_id}/encounters/bulk-randomize` (line 203)
|
||||
|
||||
**bosses.py** — boss result mutations:
|
||||
- `POST /runs/{run_id}/boss-results` (line 347)
|
||||
- `DELETE /runs/{run_id}/boss-results/{result_id}` (line 428)
|
||||
|
||||
**runs.py** — run updates/deletion:
|
||||
- `PATCH /runs/{run_id}` (line 379) — uses `_check_run_access(run, user, require_owner=run.owner_id is not None)` which skips check for unowned runs
|
||||
- `DELETE /runs/{run_id}` (line 488) — same conditional check
|
||||
|
||||
**genlockes.py** — genlocke mutations:
|
||||
- `POST /genlockes` (line 439) — no owner assigned to created genlocke or its first run
|
||||
- `PATCH /genlockes/{id}` (line 824) — no ownership check
|
||||
- `DELETE /genlockes/{id}` (line 862) — no ownership check
|
||||
- `POST /genlockes/{id}/legs/{leg_order}/advance` (line 569) — no ownership check
|
||||
- `POST /genlockes/{id}/legs` (line 894) — no ownership check
|
||||
- `DELETE /genlockes/{id}/legs/{leg_id}` (line 936) — no ownership check
|
||||
|
||||
## Approach
|
||||
|
||||
1. Add a reusable `_check_run_owner(run, user)` helper in `auth.py` or `runs.py` that raises 403 if `user.id != str(run.owner_id)` (no fallback for unowned runs — they should be read-only)
|
||||
2. Apply ownership check to ALL encounter/boss/run mutation endpoints
|
||||
3. For genlocke mutations, load the first leg's run and verify ownership against that
|
||||
4. Update `_check_run_access` to always require ownership for mutations (remove the `require_owner` conditional)
|
||||
5. When creating runs (standalone or via genlocke), set `owner_id` from the authenticated user
|
||||
|
||||
## Checklist
|
||||
|
||||
- [x] Add `_check_run_owner` helper that rejects non-owners (including unowned/legacy runs)
|
||||
- [x] Apply ownership check to all 4 encounter mutation endpoints
|
||||
- [x] Apply ownership check to both boss result mutation endpoints
|
||||
- [x] Fix `_check_run_access` to always require ownership on mutations
|
||||
- [x] Set `owner_id` on run creation in `runs.py` and `genlockes.py` (create_genlocke, advance_leg)
|
||||
- [x] Apply ownership check to all genlocke mutation endpoints (via first leg's run owner)
|
||||
- [x] Add tests for ownership enforcement (403 for non-owner, 401 for unauthenticated)
|
||||
|
||||
## Summary of Changes
|
||||
|
||||
Added `require_run_owner` helper in `auth.py` that enforces ownership on mutation endpoints:
|
||||
- Returns 403 for unowned (legacy) runs - they are now read-only
|
||||
- Returns 403 if authenticated user is not the run's owner
|
||||
|
||||
Applied ownership checks to:
|
||||
- All 4 encounter mutation endpoints (create, update, delete, bulk-randomize)
|
||||
- Both boss result mutation endpoints (create, delete)
|
||||
- Run update and delete endpoints (via `require_run_owner`)
|
||||
- All 5 genlocke mutation endpoints (update, delete, advance_leg, add_leg, remove_leg via `_check_genlocke_owner`)
|
||||
|
||||
Added `owner_id` on run creation:
|
||||
- `runs.py`: create_run already sets owner_id (verified)
|
||||
- `genlockes.py`: create_genlocke now sets owner_id on the first run
|
||||
- `genlockes.py`: advance_leg preserves owner_id from current run to new run
|
||||
|
||||
Renamed `_check_run_access` to `_check_run_read_access` (read-only visibility check) for clarity.
|
||||
|
||||
Added 22 comprehensive tests in `test_ownership.py` covering:
|
||||
- Owner can perform mutations
|
||||
- Non-owner gets 403 on mutations
|
||||
- Unauthenticated user gets 401
|
||||
- Unowned (legacy) runs reject all mutations
|
||||
- Read access preserved for public runs
|
||||
Reference in New Issue
Block a user