mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-11-09 05:24:49 +09:00
Juan DenisandClaude Opus 5
6cd2ee6b95
Plan 76 milestone A, drift/bug row 3. Five API modules had grown a private _require_admin() with *three mutually incompatible* return contracts: jobs.py -> returns the user; None means DENIED dns_cutover.py -> returns None on success; a response tuple means denied waf.py -> same as dns_cutover manifests.py -> (user, None) on success, (None, response) on denial themes.py -> same as manifests jobs.py's is the exact inverse of dns_cutover.py's, so a guard copied between the two reads identically at the call site and fails open. Nothing in the code told you which contract you were looking at. Replace all five with rbac.require_admin_user(), which raises a typed PermissionDeniedError the global ApplicationError handler already renders. A gate that raises has no return contract to get backwards. Two convergences ride along, both toward the stricter shared behaviour: - manifests.py gated on `user.role != 'admin'` where the others used `is_admin`. These are the same predicate today (is_admin is defined as role == ROLE_ADMIN), so no route changes status, but there is now one truth source rather than two. - none of the five checked is_active, so a deactivated admin still passed them; @admin_required has always checked it. The shared door checks it, so these 25 call sites now reject deactivated admins like every other admin route. The unknown-user case keeps its 403 'Admin access required' rather than becoming a 401/404: a non-admin caller should not learn from the status code whether the account behind the token still exists. Gate: tests/test_jobs.py, test_themes.py, test_waf.py, test_dns_cutover_api.py (82 passed) and -k manifest (152 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>