Procházet zdrojové kódy

Fix R02-N03: extract SessionGuard::requireAdminForm + drop SprintController::gateJsonAdmin

Form-handling controllers used to open with the same three-line block:

    $actor = SessionGuard::requireAdmin($this->users);
    if ($actor instanceof Response) return $actor;
    if (!SessionGuard::verifyCsrf($req)) return Response::text('CSRF token invalid', 403);

That's nine sites — SprintController::create / ::delete,
WorkerController::create / ::update, UserController::update / ::tombstone,
SettingsController::update, ImportController::upload / ::commit. A new
form endpoint can silently land without the CSRF or the admin gate and
the surrounding code looks fine in review. The JSON path already had a
twin (SessionGuard::requireAdminJson, SprintController::gateJsonAdmin);
the form path just lacked one.

This commit:

- Adds SessionGuard::requireAdminForm(Request, UserRepository): User|Response
  mirroring the existing requireAdminJson, but speaking the form dialect:
  redirect to /auth/login when anonymous, 403 text on non-admin, 403 text
  on CSRF mismatch.
- Replaces all nine inlined three-line blocks with a single call to the
  helper. Each call site drops from four lines to three.
- Migrates SprintController::gateJsonAdmin's eight call sites to
  SessionGuard::requireAdminJson and removes the private method — it
  was identical to the SessionGuard helper and not sprint-specific.
- Drops the now-unused App\Domain\User import in SprintController.

Net: +37 -64 lines. Tests unchanged at 340/340 (5 skipped). Spec
invariants untouched: vanilla JS, strict CSP, Twig auto-escape,
audit-row-per-mutation, capacity math under §5, audit logging rules in
§7. The 403 shape is unchanged for both paths (text response on the
form path, JSON envelope on the JSON path), so existing acceptance
behaviour is preserved exactly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa před 2 dny
rodič
revize
57f4143

+ 20 - 0
src/Auth/SessionGuard.php

@@ -268,6 +268,26 @@ final class SessionGuard
         return $user;
         return $user;
     }
     }
 
 
+    /**
+     * Form-flavoured admin gate: auth + admin + CSRF. Mirrors
+     * `requireAdminJson` but speaks the form-controller dialect — anonymous
+     * users get a redirect to /auth/login, non-admins get a 403 text
+     * response, and a CSRF mismatch gets a 403 text response. Use this
+     * inside form-handling controller actions to replace the recurring
+     * three-block boilerplate (R02-N03).
+     */
+    public static function requireAdminForm(Request $req, UserRepository $users): User|Response
+    {
+        $user = self::requireAdmin($users);
+        if ($user instanceof Response) {
+            return $user;
+        }
+        if (!self::verifyCsrf($req)) {
+            return Response::text('CSRF token invalid', 403);
+        }
+        return $user;
+    }
+
     /**
     /**
      * JSON gate for endpoints any signed-in user may hit (Phase 18:
      * JSON gate for endpoints any signed-in user may hit (Phase 18:
      * task-status writes are the first such surface). Auth + CSRF, no admin
      * task-status writes are the first such surface). Auth + CSRF, no admin

+ 2 - 8
src/Controllers/ImportController.php

@@ -87,13 +87,10 @@ final class ImportController
 
 
     public function upload(Request $req): Response
     public function upload(Request $req): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         // Validate the upload up-front; fail-closed.
         // Validate the upload up-front; fail-closed.
         if (!isset($_FILES['xlsx']) || !is_array($_FILES['xlsx'])) {
         if (!isset($_FILES['xlsx']) || !is_array($_FILES['xlsx'])) {
@@ -192,13 +189,10 @@ final class ImportController
 
 
     public function commit(Request $req, array $params): Response
     public function commit(Request $req, array $params): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
         $token = (string) ($params['token'] ?? '');
         $token = (string) ($params['token'] ?? '');
         $entry = $this->loadSessionEntry($token, $req, $actor);
         $entry = $this->loadSessionEntry($token, $req, $actor);
         if ($entry === null) {
         if ($entry === null) {

+ 1 - 4
src/Controllers/SettingsController.php

@@ -92,13 +92,10 @@ final class SettingsController
     /** POST /settings — admin only, CSRF via _csrf form field. */
     /** POST /settings — admin only, CSRF via _csrf form field. */
     public function update(Request $req): Response
     public function update(Request $req): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $this->pdo->beginTransaction();
         $this->pdo->beginTransaction();
         try {
         try {

+ 10 - 36
src/Controllers/SprintController.php

@@ -14,7 +14,6 @@ namespace App\Controllers;
 
 
 use App\Auth\SessionGuard;
 use App\Auth\SessionGuard;
 use App\Domain\SprintWeek;
 use App\Domain\SprintWeek;
-use App\Domain\User;
 use App\Http\Request;
 use App\Http\Request;
 use App\Http\Response;
 use App\Http\Response;
 use App\Http\View;
 use App\Http\View;
@@ -90,13 +89,10 @@ final class SprintController
     /** POST /sprints — create sprint + materialise weeks in one tx. */
     /** POST /sprints — create sprint + materialise weeks in one tx. */
     public function create(Request $req): Response
     public function create(Request $req): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $name     = trim($req->postString('name'));
         $name     = trim($req->postString('name'));
         $start    = $req->postString('start_date');
         $start    = $req->postString('start_date');
@@ -346,7 +342,7 @@ final class SprintController
     /** PATCH /sprints/{id} — JSON — update name / dates / reserve_fraction. */
     /** PATCH /sprints/{id} — JSON — update name / dates / reserve_fraction. */
     public function updateMeta(Request $req, array $params): Response
     public function updateMeta(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -507,7 +503,7 @@ final class SprintController
     /** POST /sprints/{id}/weeks — JSON — resize the week set. */
     /** POST /sprints/{id}/weeks — JSON — resize the week set. */
     public function replaceWeeks(Request $req, array $params): Response
     public function replaceWeeks(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -586,7 +582,7 @@ final class SprintController
     /** POST /sprints/{id}/workers — JSON — add a worker to the sprint. */
     /** POST /sprints/{id}/workers — JSON — add a worker to the sprint. */
     public function addWorker(Request $req, array $params): Response
     public function addWorker(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -646,7 +642,7 @@ final class SprintController
     /** DELETE /sprints/{id}/workers/{sw_id} — JSON — remove a worker from the sprint. */
     /** DELETE /sprints/{id}/workers/{sw_id} — JSON — remove a worker from the sprint. */
     public function removeWorker(Request $req, array $params): Response
     public function removeWorker(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -702,7 +698,7 @@ final class SprintController
     /** POST /sprints/{id}/workers/reorder — JSON — apply an ordering. */
     /** POST /sprints/{id}/workers/reorder — JSON — apply an ordering. */
     public function reorderWorkers(Request $req, array $params): Response
     public function reorderWorkers(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -759,7 +755,7 @@ final class SprintController
     /** PATCH /sprints/{id}/workers/{sw_id} — JSON — edit RTB. */
     /** PATCH /sprints/{id}/workers/{sw_id} — JSON — edit RTB. */
     public function updateWorker(Request $req, array $params): Response
     public function updateWorker(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -803,7 +799,7 @@ final class SprintController
     /** PATCH /sprints/{id}/week-cells — JSON — batch upsert of sprint_worker_days. */
     /** PATCH /sprints/{id}/week-cells — JSON — batch upsert of sprint_worker_days. */
     public function updateWeekCells(Request $req, array $params): Response
     public function updateWeekCells(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -925,7 +921,7 @@ final class SprintController
      */
      */
     public function updateWeekDays(Request $req, array $params): Response
     public function updateWeekDays(Request $req, array $params): Response
     {
     {
-        $gate = $this->gateJsonAdmin($req);
+        $gate = SessionGuard::requireAdminJson($req, $this->users);
         if ($gate instanceof Response) {
         if ($gate instanceof Response) {
             return $gate;
             return $gate;
         }
         }
@@ -1017,13 +1013,10 @@ final class SprintController
      */
      */
     public function delete(Request $req, array $params): Response
     public function delete(Request $req, array $params): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $sprintId = (int) $params['id'];
         $sprintId = (int) $params['id'];
         $sprint   = $this->sprints->find($sprintId);
         $sprint   = $this->sprints->find($sprintId);
@@ -1162,25 +1155,6 @@ final class SprintController
     // Shared helpers
     // Shared helpers
     // ------------------------------------------------------------------
     // ------------------------------------------------------------------
 
 
-    /**
-     * Admin gate for JSON endpoints. Returns the signed-in User on success,
-     * or an `Response::err(...)` JSON envelope on failure. Also enforces CSRF.
-     */
-    private function gateJsonAdmin(Request $req): User|Response
-    {
-        $user = SessionGuard::currentUser($this->users);
-        if ($user === null) {
-            return Response::err('unauthenticated', 'Sign in required', 401);
-        }
-        if (!$user->isAdmin) {
-            return Response::err('forbidden', 'Admin access required', 403);
-        }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::err('csrf', 'CSRF token invalid', 403);
-        }
-        return $user;
-    }
-
     private static function isIsoDate(string $s): bool
     private static function isIsoDate(string $s): bool
     {
     {
         $d = DateTimeImmutable::createFromFormat('Y-m-d', $s);
         $d = DateTimeImmutable::createFromFormat('Y-m-d', $s);

+ 2 - 8
src/Controllers/UserController.php

@@ -70,13 +70,10 @@ final class UserController
     /** POST /users/{id} — form — toggle is_admin. */
     /** POST /users/{id} — form — toggle is_admin. */
     public function update(Request $req, array $params): Response
     public function update(Request $req, array $params): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $id = (int) $params['id'];
         $id = (int) $params['id'];
         $target = $this->users->find($id);
         $target = $this->users->find($id);
@@ -188,13 +185,10 @@ final class UserController
      */
      */
     public function tombstone(Request $req, array $params): Response
     public function tombstone(Request $req, array $params): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $id = (int) $params['id'];
         $id = (int) $params['id'];
         $target = $this->users->find($id);
         $target = $this->users->find($id);

+ 2 - 8
src/Controllers/WorkerController.php

@@ -61,13 +61,10 @@ final class WorkerController
     /** POST /workers — create a worker. */
     /** POST /workers — create a worker. */
     public function create(Request $req): Response
     public function create(Request $req): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $name    = trim($req->postString('name'));
         $name    = trim($req->postString('name'));
         $rtbRaw  = $req->postString('default_rtb');
         $rtbRaw  = $req->postString('default_rtb');
@@ -110,13 +107,10 @@ final class WorkerController
     /** POST /workers/{id} (form) — update a worker's editable fields. */
     /** POST /workers/{id} (form) — update a worker's editable fields. */
     public function update(Request $req, array $params): Response
     public function update(Request $req, array $params): Response
     {
     {
-        $actor = SessionGuard::requireAdmin($this->users);
+        $actor = SessionGuard::requireAdminForm($req, $this->users);
         if ($actor instanceof Response) {
         if ($actor instanceof Response) {
             return $actor;
             return $actor;
         }
         }
-        if (!SessionGuard::verifyCsrf($req)) {
-            return Response::text('CSRF token invalid', 403);
-        }
 
 
         $id = (int) $params['id'];
         $id = (int) $params['id'];
         $existing = $this->workers->find($id);
         $existing = $this->workers->find($id);