Sfoglia il codice sorgente

Phase 9: users management page (promote / demote admin)

The first-login bootstrap was the only way to get an admin; if the
admin demoted themselves there was no UI to recover. This phase adds:

GET  /users          (admin) — table of every user with email, display
                              name, last login, and an Admin checkbox.
POST /users/{id}     (admin, CSRF) — toggle is_admin.

Server-side guardrails (both surface as redirect ?error= codes and
write nothing to the DB):
  self_demote — you cannot demote yourself
  last_admin  — cannot demote the only remaining admin
The checkbox for the current admin is also rendered disabled in the
view as defense-in-depth, but the server check is what's load-bearing.

Toggles audit as UPDATE on entity_type=user with full before/after
snapshots. No-op submits flash "noop" and write zero rows (the audit
no-op rule catches them, but we also short-circuit before the tx).

Code:
- UserRepository: new all() (ORDER BY LOWER(email)), countAdmins(),
  setAdmin(int $id, bool) returning {before, after}.
- UserController: index + update + a pure static demoteGuardrail()
  helper extracted so tests can pin the rule matrix without spinning
  up SessionGuard / PDO. The instance method calls the static.
- views/users/index.php: simple table; current row gets a "you" chip
  and a disabled checkbox when self-admin; flash + error banners
  share the workers-page idiom.
- public/index.php wires the controller + GET/POST routes.
- views/layout.php gains a "Users" admin nav link between Workers and
  New sprint.

Tests:
- UserRepositoryTest gains coverage for all() (alphabetical sort),
  countAdmins() (incl. forceAdmin path), setAdmin() before/after.
- UserControllerTest data-providers the 8-case guardrail matrix:
  self-demote, last-admin, demote-with-spare-admins, promotion paths,
  no-op cases.

Suite: 74 tests, 138 assertions — all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
achiappa 2 settimane fa
parent
commit
f7f5db5104

+ 5 - 0
public/index.php

@@ -9,6 +9,7 @@ use App\Controllers\AuditController;
 use App\Controllers\AuthController;
 use App\Controllers\SprintController;
 use App\Controllers\TaskController;
+use App\Controllers\UserController;
 use App\Controllers\WorkerController;
 use App\Db\Connection;
 use App\Db\Migrator;
@@ -103,6 +104,7 @@ $taskCtrl       = new TaskController(
     $tasks, $taskAssign, $workers, $audit,
 );
 $auditCtrl      = new AuditController($users, $auditRepo, $view);
+$userCtrl       = new UserController($pdo, $users, $audit, $view);
 
 // ---------------------------------------------------------------------------
 // Routing
@@ -143,6 +145,9 @@ $router->get('/workers',         $workerCtrl->index(...));
 $router->post('/workers',        $workerCtrl->create(...));
 $router->post('/workers/{id}',   $workerCtrl->update(...));
 
+$router->get('/users',           $userCtrl->index(...));
+$router->post('/users/{id}',     $userCtrl->update(...));
+
 $router->get('/sprints/new',              $sprintCtrl->newForm(...));
 $router->post('/sprints',                 $sprintCtrl->create(...));
 $router->get('/sprints/{id}',             $sprintCtrl->show(...));

+ 136 - 0
src/Controllers/UserController.php

@@ -0,0 +1,136 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Controllers;
+
+use App\Auth\SessionGuard;
+use App\Http\Request;
+use App\Http\Response;
+use App\Http\View;
+use App\Repositories\UserRepository;
+use App\Services\AuditLogger;
+use PDO;
+use Throwable;
+
+/**
+ * /users — admin-only page for promoting / demoting admin status.
+ *
+ * Guardrails (server-side, both surface as ?error= codes):
+ *   - You cannot demote yourself.
+ *   - You cannot demote the last remaining admin.
+ *
+ * No delete — users are never deleted; they'd orphan audit rows that
+ * reference them by id.
+ */
+final class UserController
+{
+    public function __construct(
+        private readonly PDO            $pdo,
+        private readonly UserRepository $users,
+        private readonly AuditLogger    $audit,
+        private readonly View           $view,
+    ) {
+    }
+
+    /** GET /users */
+    public function index(Request $req): Response
+    {
+        $actor = SessionGuard::requireAdmin($this->users);
+        if ($actor instanceof Response) {
+            return $actor;
+        }
+
+        return Response::html($this->view->render('users/index', [
+            'title'       => 'Users',
+            'currentUser' => $actor,
+            'csrfToken'   => SessionGuard::csrfToken(),
+            'users'       => $this->users->all(),
+            'flash'       => $req->queryString('flash'),
+            'error'       => $req->queryString('error'),
+        ]));
+    }
+
+    /** POST /users/{id} — form — toggle is_admin. */
+    public function update(Request $req, array $params): Response
+    {
+        $actor = SessionGuard::requireAdmin($this->users);
+        if ($actor instanceof Response) {
+            return $actor;
+        }
+        if (!SessionGuard::verifyCsrf($req)) {
+            return Response::text('CSRF token invalid', 403);
+        }
+
+        $id = (int) $params['id'];
+        $target = $this->users->find($id);
+        if ($target === null) {
+            return Response::redirect('/users?error=not_found');
+        }
+
+        // Checkbox is absent from $_POST when unchecked.
+        $targetIsAdmin = isset($req->post['is_admin'])
+            && (string) $req->post['is_admin'] !== '0';
+
+        if ($targetIsAdmin === $target->isAdmin) {
+            return Response::redirect('/users?flash=noop');
+        }
+
+        $error = self::demoteGuardrail(
+            actorId:      $actor->id,
+            targetId:     $target->id,
+            targetWasAdmin: $target->isAdmin,
+            targetWillBeAdmin: $targetIsAdmin,
+            totalAdmins:  $this->users->countAdmins(),
+        );
+        if ($error !== null) {
+            return Response::redirect('/users?error=' . $error);
+        }
+
+        $this->pdo->beginTransaction();
+        try {
+            $result = $this->users->setAdmin($id, $targetIsAdmin);
+            $this->audit->recordForRequest(
+                'UPDATE', 'user', $id,
+                $result['before']->toAuditSnapshot(),
+                $result['after']->toAuditSnapshot(),
+                $req, $actor,
+            );
+            $this->pdo->commit();
+        } catch (Throwable) {
+            $this->pdo->rollBack();
+            return Response::redirect('/users?error=db_error');
+        }
+
+        return Response::redirect('/users?flash=' . ($targetIsAdmin ? 'promoted' : 'demoted'));
+    }
+
+    /**
+     * Pure guardrail check for admin demotions. Returns null when the action
+     * is allowed, or a short error code for the redirect query string.
+     *
+     *  self_demote  — the actor is trying to demote themselves
+     *  last_admin   — demoting the only remaining admin
+     *
+     * Extracted so tests can hit the rules without SessionGuard/PDO setup.
+     */
+    public static function demoteGuardrail(
+        int $actorId,
+        int $targetId,
+        bool $targetWasAdmin,
+        bool $targetWillBeAdmin,
+        int $totalAdmins,
+    ): ?string {
+        // Only enforce when the change is an admin demotion.
+        if (!$targetWasAdmin || $targetWillBeAdmin) {
+            return null;
+        }
+        if ($targetId === $actorId) {
+            return 'self_demote';
+        }
+        if ($totalAdmins <= 1) {
+            return 'last_admin';
+        }
+        return null;
+    }
+}

+ 39 - 0
src/Repositories/UserRepository.php

@@ -94,6 +94,45 @@ final class UserRepository
         return ['user' => $after, 'before' => $existing];
     }
 
+    /** @return list<User> ordered by LOWER(email) ASC */
+    public function all(): array
+    {
+        $stmt = $this->pdo->query('SELECT * FROM users ORDER BY LOWER(email) ASC');
+        $out = [];
+        foreach ($stmt as $row) {
+            $out[] = self::hydrate($row);
+        }
+        return $out;
+    }
+
+    /** How many admin users currently exist. */
+    public function countAdmins(): int
+    {
+        return (int) $this->pdo
+            ->query('SELECT COUNT(*) FROM users WHERE is_admin = 1')
+            ->fetchColumn();
+    }
+
+    /**
+     * Force is_admin to the given value. Returns before/after so the caller
+     * can audit. Guardrails (cannot demote self, cannot remove the last
+     * admin) live in the controller — this repo only does the SQL.
+     *
+     * @return array{before: User, after: User}
+     */
+    public function setAdmin(int $id, bool $isAdmin): array
+    {
+        $before = $this->find($id);
+        if ($before === null) {
+            throw new \RuntimeException("User {$id} not found");
+        }
+        $this->pdo
+            ->prepare('UPDATE users SET is_admin = ? WHERE id = ?')
+            ->execute([$isAdmin ? 1 : 0, $id]);
+        $after = $this->find($id) ?? $before;
+        return ['before' => $before, 'after' => $after];
+    }
+
     /**
      * @param array<string,mixed> $row
      */

+ 49 - 0
tests/Controllers/UserControllerTest.php

@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controllers;
+
+use App\Controllers\UserController;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * The demotion guardrails are extracted into a pure static method; this
+ * test pins the rule matrix without any SessionGuard / PDO setup.
+ */
+final class UserControllerTest extends TestCase
+{
+    /** @return list<array{int,int,bool,bool,int,?string,string}> */
+    public static function guardrailCases(): array
+    {
+        // [actorId, targetId, wasAdmin, willBeAdmin, totalAdmins, expected, label]
+        return [
+            [1, 1, true,  false, 2, 'self_demote', 'self demote, 2 admins'],
+            [1, 1, true,  false, 1, 'self_demote', 'self demote, only admin — self wins priority'],
+            [1, 2, true,  false, 1, 'last_admin',  'demote the only admin (different user)'],
+            [1, 2, true,  false, 2, null,          'demote someone else when another admin remains'],
+            [1, 2, false, true,  1, null,          'promote — never blocked'],
+            [1, 2, true,  true,  1, null,          'no-op — never blocked'],
+            [1, 2, false, false, 1, null,          'no-op on non-admin — never blocked'],
+            [1, 1, false, true,  1, null,          'actor promoting themselves? odd but not blocked'],
+        ];
+    }
+
+    #[DataProvider('guardrailCases')]
+    public function testDemoteGuardrail(
+        int $actor,
+        int $target,
+        bool $was,
+        bool $will,
+        int $total,
+        ?string $expected,
+        string $label,
+    ): void {
+        $this->assertSame(
+            $expected,
+            UserController::demoteGuardrail($actor, $target, $was, $will, $total),
+            $label,
+        );
+    }
+}

+ 52 - 0
tests/Repositories/UserRepositoryTest.php

@@ -98,4 +98,56 @@ final class UserRepositoryTest extends TestCase
         $users->upsertFromOidc('oid-1', 'a@x', 'A', false);
         $this->assertSame(2, $users->count());
     }
+
+    // ------------------------------------------------------------------
+    // Phase 9: users management page helpers
+    // ------------------------------------------------------------------
+
+    public function testAllReturnsEveryUserOrderedByEmail(): void
+    {
+        $pdo   = $this->makeDb();
+        $users = new UserRepository($pdo);
+
+        $users->upsertFromOidc('oid-c', 'carol@x',  'Carol', true);
+        $users->upsertFromOidc('oid-a', 'alice@x',  'Alice', false);
+        $users->upsertFromOidc('oid-b', 'BOB@x',    'Bob',   false);
+
+        $all = $users->all();
+        $this->assertCount(3, $all);
+        $this->assertSame(['alice@x', 'BOB@x', 'carol@x'], array_map(fn($u) => $u->email, $all));
+    }
+
+    public function testCountAdmins(): void
+    {
+        $pdo   = $this->makeDb();
+        $users = new UserRepository($pdo);
+
+        $this->assertSame(0, $users->countAdmins());
+        $users->upsertFromOidc('oid-a', 'a@x', 'A', true);
+        $this->assertSame(1, $users->countAdmins());
+        $users->upsertFromOidc('oid-b', 'b@x', 'B', false);
+        $this->assertSame(1, $users->countAdmins());
+        $users->upsertFromOidc('oid-c', 'c@x', 'C', false, true); // forceAdmin
+        $this->assertSame(2, $users->countAdmins());
+    }
+
+    public function testSetAdminTogglesAndReportsDiff(): void
+    {
+        $pdo   = $this->makeDb();
+        $users = new UserRepository($pdo);
+
+        $users->upsertFromOidc('oid-a', 'a@x', 'A', true);
+        $users->upsertFromOidc('oid-b', 'b@x', 'B', false);
+
+        $bob = $users->findByOid('oid-b');
+        $r = $users->setAdmin($bob->id, true);
+        $this->assertFalse($r['before']->isAdmin);
+        $this->assertTrue ($r['after']->isAdmin);
+        $this->assertSame(2, $users->countAdmins());
+
+        $r = $users->setAdmin($bob->id, false);
+        $this->assertTrue ($r['before']->isAdmin);
+        $this->assertFalse($r['after']->isAdmin);
+        $this->assertSame(1, $users->countAdmins());
+    }
 }

+ 1 - 0
views/layout.php

@@ -29,6 +29,7 @@ $csrfToken   = $csrfToken   ?? '';
                     <a href="/" class="text-slate-600 hover:text-slate-900 hover:underline">Sprints</a>
                     <?php if ($currentUser->isAdmin): ?>
                         <a href="/workers" class="text-slate-600 hover:text-slate-900 hover:underline">Workers</a>
+                        <a href="/users"   class="text-slate-600 hover:text-slate-900 hover:underline">Users</a>
                         <a href="/sprints/new" class="text-slate-600 hover:text-slate-900 hover:underline">New sprint</a>
                         <a href="/audit" class="text-slate-600 hover:text-slate-900 hover:underline">Audit log</a>
                     <?php endif; ?>

+ 93 - 0
views/users/index.php

@@ -0,0 +1,93 @@
+<?php
+/** @var list<\App\Domain\User> $users */
+/** @var \App\Domain\User        $currentUser */
+/** @var string                  $csrfToken */
+/** @var string                  $flash */
+/** @var string                  $error */
+use function App\Http\e;
+
+$errorMessages = [
+    'self_demote' => 'You cannot demote yourself — ask another admin.',
+    'last_admin'  => 'Cannot demote the last remaining admin.',
+    'not_found'   => 'User not found.',
+    'db_error'    => 'Could not save. Try again.',
+];
+$flashMessages = [
+    'promoted' => 'Admin granted.',
+    'demoted'  => 'Admin revoked.',
+    'noop'     => 'Nothing changed.',
+];
+?>
+<section class="space-y-6">
+    <div>
+        <h1 class="text-2xl font-semibold tracking-tight">Users</h1>
+        <p class="text-slate-600 text-sm mt-1 max-w-prose">
+            Everyone who has ever signed in. Toggle admin status here; you
+            cannot demote yourself or the last admin. Users are never deleted
+            — inactive accounts simply stop signing in.
+        </p>
+    </div>
+
+    <?php if ($error !== '' && isset($errorMessages[$error])): ?>
+        <div class="rounded-md border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-800">
+            <?= e($errorMessages[$error]) ?>
+        </div>
+    <?php endif; ?>
+    <?php if ($flash !== '' && isset($flashMessages[$flash])): ?>
+        <div class="rounded-md border border-green-200 bg-green-50 px-4 py-3 text-sm text-green-800">
+            <?= e($flashMessages[$flash]) ?>
+        </div>
+    <?php endif; ?>
+
+    <div class="rounded-lg border bg-white overflow-hidden">
+        <?php if ($users === []): ?>
+            <div class="p-8 text-center text-slate-500 text-sm">No users yet.</div>
+        <?php else: ?>
+            <table class="min-w-full text-sm">
+                <thead class="bg-slate-50 text-slate-600 text-xs uppercase tracking-wider">
+                    <tr>
+                        <th class="text-left px-4 py-2 font-semibold">Email</th>
+                        <th class="text-left px-4 py-2 font-semibold">Display name</th>
+                        <th class="text-left px-4 py-2 font-semibold">Last login (UTC)</th>
+                        <th class="text-left px-4 py-2 font-semibold">Admin</th>
+                        <th class="text-right px-4 py-2 font-semibold">&nbsp;</th>
+                    </tr>
+                </thead>
+                <tbody class="divide-y divide-slate-100">
+                    <?php foreach ($users as $u): $isSelf = $u->id === $currentUser->id; ?>
+                        <tr>
+                            <form method="post" action="/users/<?= (int) $u->id ?>" class="contents">
+                                <input type="hidden" name="_csrf" value="<?= e($csrfToken) ?>">
+                                <td class="px-4 py-2 font-mono text-xs">
+                                    <?= e($u->email) ?>
+                                    <?php if ($isSelf): ?>
+                                        <span class="ml-1 inline-block px-1.5 py-0.5 text-[10px] font-semibold uppercase tracking-wider bg-slate-100 text-slate-700 rounded">you</span>
+                                    <?php endif; ?>
+                                </td>
+                                <td class="px-4 py-2"><?= e($u->displayName) ?></td>
+                                <td class="px-4 py-2 text-slate-500 font-mono text-xs">
+                                    <?= $u->lastLoginAt !== null ? e($u->lastLoginAt) : '<span class="text-slate-400">—</span>' ?>
+                                </td>
+                                <td class="px-4 py-2">
+                                    <label class="inline-flex items-center gap-2">
+                                        <input name="is_admin" type="checkbox" value="1"
+                                               <?= $u->isAdmin ? 'checked' : '' ?>
+                                               <?= $isSelf && $u->isAdmin ? 'disabled title="You cannot demote yourself"' : '' ?>
+                                               class="rounded border-slate-300">
+                                        <span class="text-slate-600">admin</span>
+                                    </label>
+                                </td>
+                                <td class="px-4 py-2 text-right">
+                                    <button type="submit"
+                                            class="rounded-md border border-slate-300 bg-white text-slate-700 px-3 py-1 text-sm hover:bg-slate-100">
+                                        Save
+                                    </button>
+                                </td>
+                            </form>
+                        </tr>
+                    <?php endforeach; ?>
+                </tbody>
+            </table>
+        <?php endif; ?>
+    </div>
+</section>