Bladeren bron

Fix R01-N06: throttle local-admin login by (ip, email)

The /auth/local path had no rate limiting — an attacker could brute-
force the password as fast as the server could respond. The audit log
recorded LOGIN_FAILED rows but nothing blocked the next attempt. With
R01-N01 already enforcing a hash-only credential, throttling is the
remaining defence on the local-admin path (OIDC has Entra's own).

New tiny migration `005_auth_throttle.sql` adds an `auth_throttle` table
keyed by (ip_address, email). The new `AuthThrottleRepository`:

  - lockoutFor(ip, email, now): current lock end if any
  - recordFailure(ip, email, now): increment counter, return new lock
  - clear(ip, email): drops the row on success
  - purgeExpired(now): housekeeping helper (not yet wired)

Policy is in the static `computeLockout(attempts, now)` so tests can
pin the matrix without DB:

  1-4   attempts → no lock
  5-9             → +5 min
  10-19           → +30 min
  20+             → +1 hour

The counter naturally resets if no failure occurs for 15 minutes, so an
honest user who later fat-fingers the password isn't penalised forever.
A successful sign-in clears the bucket outright. Email is canonicalised
(lowercased + trimmed) so case variations don't dodge the bucket; IPs
keyed verbatim, so different attacker IPs don't share a lock.

`AuthController::loginLocal` checks the lock BEFORE calling
`LocalAdmin::verify`, so a slow `password_verify()` can't be turned
into an oracle by a still-locked attacker. On failure it records a
LOGIN_FAILED audit row with a `reason` of either
`local_admin_throttled_until_<iso>` (lock kicked in) or the existing
`local_admin_credential_mismatch` (counter incremented but below the
threshold). On success it clears the bucket. The form template grows
a single amber "Too many failed attempts" notice rendered when the
controller redirects to `/auth/local?throttled=1`.

Tests: new `tests/Repositories/AuthThrottleRepositoryTest.php` with 13
cases covers the no-lock, 5/10/20 escalation thresholds, lockout
expiry boundary (4:59 still locked, 5:00 free), idle-window reset
(stale → counter restart), within-window persistence, clear(), case-
insensitive email bucketing, IP isolation, full computeLockout matrix,
and purgeExpired's selectivity. Time is injected so no test sleeps.
Tests: 176 / 484 (was 163 / 417).
chiappa 2 dagen geleden
bovenliggende
commit
e295432bef

+ 23 - 0
migrations/005_auth_throttle.sql

@@ -0,0 +1,23 @@
+-- R01-N06: persistent per-(ip, email) throttle for /auth/local.
+--
+-- The local-admin sign-in path had no rate limiting — an attacker could
+-- brute-force the password as fast as the server could respond. Audit rows
+-- captured each attempt but nothing blocked the next one.
+--
+-- One row per (ip_address, email) pair seen with a recent failure. Counter
+-- is reset on a successful login (DELETE) and naturally rolls over after a
+-- 15-minute idle window. Lock thresholds are policy code in
+-- AuthThrottleRepository::computeLockout(); the table stores only counts +
+-- timestamps.
+
+CREATE TABLE auth_throttle (
+    ip_address        TEXT NOT NULL,
+    email             TEXT NOT NULL,
+    attempts          INTEGER NOT NULL DEFAULT 0,
+    first_failure_at  TEXT NOT NULL,
+    last_failure_at   TEXT NOT NULL,
+    locked_until      TEXT,
+    PRIMARY KEY (ip_address, email)
+);
+
+CREATE INDEX idx_auth_throttle_locked ON auth_throttle(locked_until);

+ 3 - 1
public/index.php

@@ -21,6 +21,7 @@ use App\Http\Router;
 use App\Http\View;
 use App\Repositories\AppSettingsRepository;
 use App\Repositories\AuditRepository;
+use App\Repositories\AuthThrottleRepository;
 use App\Repositories\SprintRepository;
 use App\Repositories\SprintWeekRepository;
 use App\Repositories\SprintWorkerDayRepository;
@@ -102,8 +103,9 @@ $tasks          = new TaskRepository($pdo);
 $taskAssign     = new TaskAssignmentRepository($pdo);
 $auditRepo      = new AuditRepository($pdo);
 $appSettings    = new AppSettingsRepository($pdo);
+$authThrottle   = new AuthThrottleRepository($pdo);
 $audit          = new AuditLogger($pdo);
-$auth           = new AuthController($pdo, $users, $audit, $view);
+$auth           = new AuthController($pdo, $users, $audit, $view, $authThrottle);
 $workerCtrl     = new WorkerController($pdo, $users, $workers, $audit, $view);
 $sprintCtrl     = new SprintController(
     $pdo, $users, $sprints, $sprintWeeks, $sprintWorkers, $swDays,

+ 32 - 5
src/Controllers/AuthController.php

@@ -10,18 +10,22 @@ use App\Auth\SessionGuard;
 use App\Http\Request;
 use App\Http\Response;
 use App\Http\View;
+use App\Repositories\AuthThrottleRepository;
 use App\Repositories\UserRepository;
 use App\Services\AuditLogger;
+use DateTimeImmutable;
+use DateTimeZone;
 use PDO;
 use Throwable;
 
 final class AuthController
 {
     public function __construct(
-        private readonly PDO            $pdo,
-        private readonly UserRepository $users,
-        private readonly AuditLogger    $audit,
-        private readonly View           $view,
+        private readonly PDO                    $pdo,
+        private readonly UserRepository         $users,
+        private readonly AuditLogger            $audit,
+        private readonly View                   $view,
+        private readonly AuthThrottleRepository $throttle,
     ) {
     }
 
@@ -180,13 +184,15 @@ final class AuthController
             return Response::text('Not Found', 404);
         }
         SessionGuard::start();
-        $error = $req->queryString('error') === '1';
+        $error     = $req->queryString('error') === '1';
+        $throttled = $req->queryString('throttled') === '1';
         return Response::html($this->view->render('auth/local', [
             'title'        => 'Local sign-in',
             'currentUser'  => null,
             'csrfToken'    => SessionGuard::csrfToken(),
             'email'        => LocalAdmin::email(),
             'error'        => $error,
+            'throttled'    => $throttled,
         ]));
     }
 
@@ -207,11 +213,32 @@ final class AuthController
             ? (string) $req->post['password']
             : '';
 
+        $ip  = $req->ip();
+        $now = new DateTimeImmutable('now', new DateTimeZone('UTC'));
+
+        // R01-N06: refuse and audit while a lockout is active. We do this
+        // BEFORE password verification so a slow `password_verify()` can't
+        // be turned into an oracle by a still-locked attacker.
+        $lockedUntil = $this->throttle->lockoutFor($ip, $email, $now);
+        if ($lockedUntil !== null) {
+            $this->logFailure(
+                $req,
+                'local_admin_throttled_until_'
+                    . $lockedUntil->format('Y-m-d\TH:i:s\Z')
+            );
+            return Response::redirect('/auth/local?throttled=1');
+        }
+
         if (!LocalAdmin::verify($email, $password)) {
+            $this->throttle->recordFailure($ip, $email, $now);
             $this->logFailure($req, 'local_admin_credential_mismatch');
             return Response::redirect('/auth/local?error=1');
         }
 
+        // Clear the (ip, email) bucket on success so the next bad password
+        // doesn't lock out a legitimate operator who just signed in.
+        $this->throttle->clear($ip, $email);
+
         $this->pdo->beginTransaction();
         try {
             $isFirstUser = $this->users->count() === 0;

+ 177 - 0
src/Repositories/AuthThrottleRepository.php

@@ -0,0 +1,177 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Repositories;
+
+use DateTimeImmutable;
+use DateTimeZone;
+use PDO;
+
+/**
+ * Per-(ip, email) brute-force throttle for the local-admin login path
+ * (R01-N06). The table is the single source of truth — process restarts
+ * and parallel workers see the same counter.
+ *
+ * Policy is in `computeLockout()`:
+ *   1-4 attempts within the window → no lock
+ *   5-9                            → lock 5 minutes
+ *   10-19                          → lock 30 minutes
+ *   20+                            → lock 60 minutes
+ *
+ * The window is `WINDOW_SECONDS` (15 min) of *no failures*. After a
+ * quiet stretch the counter resets to 1 on the next failure, so an
+ * honest user who later fat-fingers their password isn't penalised
+ * forever. A successful sign-in clears the row outright.
+ *
+ * Time is injected by the caller so unit tests don't have to sleep.
+ */
+final class AuthThrottleRepository
+{
+    public const WINDOW_SECONDS = 900;
+
+    public function __construct(private readonly PDO $pdo)
+    {
+    }
+
+    /**
+     * Returns the active lock end time for ($ip, $email) — or null if
+     * there is no row, or the row's lock has already expired relative
+     * to $now.
+     */
+    public function lockoutFor(string $ip, string $email, DateTimeImmutable $now): ?DateTimeImmutable
+    {
+        $row = $this->fetch($ip, $email);
+        if ($row === null || $row['locked_until'] === null) {
+            return null;
+        }
+        $until = self::parse((string) $row['locked_until']);
+        return $until > $now ? $until : null;
+    }
+
+    /**
+     * Increment the failure counter and (re)compute the lockout. Returns
+     * the new lock end (null when below the first threshold).
+     */
+    public function recordFailure(string $ip, string $email, DateTimeImmutable $now): ?DateTimeImmutable
+    {
+        $row = $this->fetch($ip, $email);
+
+        $attempts = 1;
+        $first    = $now;
+        if ($row !== null) {
+            $last     = self::parse((string) $row['last_failure_at']);
+            $idleSecs = $now->getTimestamp() - $last->getTimestamp();
+            if ($idleSecs > self::WINDOW_SECONDS) {
+                // Window stale → start a fresh counter.
+                $attempts = 1;
+                $first    = $now;
+            } else {
+                $attempts = ((int) $row['attempts']) + 1;
+                $first    = self::parse((string) $row['first_failure_at']);
+            }
+        }
+
+        $lockedUntil = self::computeLockout($attempts, $now);
+        $this->upsert($ip, $email, $attempts, $first, $now, $lockedUntil);
+        return $lockedUntil;
+    }
+
+    /** Drop the throttle row on a successful sign-in. */
+    public function clear(string $ip, string $email): void
+    {
+        $stmt = $this->pdo->prepare(
+            'DELETE FROM auth_throttle WHERE ip_address = ? AND email = ?'
+        );
+        $stmt->execute([self::keyIp($ip), self::keyEmail($email)]);
+    }
+
+    /** Housekeeping helper; not wired into a cron yet. */
+    public function purgeExpired(DateTimeImmutable $olderThan): int
+    {
+        $stmt = $this->pdo->prepare(
+            'DELETE FROM auth_throttle
+             WHERE last_failure_at < ?
+               AND (locked_until IS NULL OR locked_until <= ?)'
+        );
+        $stmt->execute([self::format($olderThan), self::format($olderThan)]);
+        return $stmt->rowCount();
+    }
+
+    /**
+     * Pure policy: maps the post-increment attempt count to a lock-end
+     * time. Extracted so tests can pin the matrix without DB.
+     */
+    public static function computeLockout(int $attempts, DateTimeImmutable $now): ?DateTimeImmutable
+    {
+        if ($attempts < 5) {
+            return null;
+        }
+        if ($attempts < 10) {
+            return $now->modify('+5 minutes');
+        }
+        if ($attempts < 20) {
+            return $now->modify('+30 minutes');
+        }
+        return $now->modify('+1 hour');
+    }
+
+    /** @return array<string,mixed>|null */
+    private function fetch(string $ip, string $email): ?array
+    {
+        $stmt = $this->pdo->prepare(
+            'SELECT * FROM auth_throttle WHERE ip_address = ? AND email = ?'
+        );
+        $stmt->execute([self::keyIp($ip), self::keyEmail($email)]);
+        $row = $stmt->fetch();
+        return is_array($row) ? $row : null;
+    }
+
+    private function upsert(
+        string $ip,
+        string $email,
+        int $attempts,
+        DateTimeImmutable $first,
+        DateTimeImmutable $last,
+        ?DateTimeImmutable $lockedUntil,
+    ): void {
+        $stmt = $this->pdo->prepare(
+            'INSERT INTO auth_throttle
+                 (ip_address, email, attempts, first_failure_at, last_failure_at, locked_until)
+             VALUES (?, ?, ?, ?, ?, ?)
+             ON CONFLICT(ip_address, email) DO UPDATE SET
+                 attempts         = excluded.attempts,
+                 first_failure_at = excluded.first_failure_at,
+                 last_failure_at  = excluded.last_failure_at,
+                 locked_until     = excluded.locked_until'
+        );
+        $stmt->execute([
+            self::keyIp($ip),
+            self::keyEmail($email),
+            $attempts,
+            self::format($first),
+            self::format($last),
+            $lockedUntil === null ? null : self::format($lockedUntil),
+        ]);
+    }
+
+    private static function keyIp(string $ip): string
+    {
+        return trim($ip);
+    }
+
+    private static function keyEmail(string $email): string
+    {
+        return strtolower(trim($email));
+    }
+
+    private static function format(DateTimeImmutable $dt): string
+    {
+        return $dt->setTimezone(new DateTimeZone('UTC'))->format('Y-m-d\TH:i:s\Z');
+    }
+
+    private static function parse(string $s): DateTimeImmutable
+    {
+        return new DateTimeImmutable($s);
+    }
+}

+ 223 - 0
tests/Repositories/AuthThrottleRepositoryTest.php

@@ -0,0 +1,223 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Repositories;
+
+use App\Repositories\AuthThrottleRepository;
+use App\Tests\TestCase;
+use DateTimeImmutable;
+use DateTimeZone;
+
+/**
+ * R01-N06: persistent (ip, email) throttle for /auth/local. Tests pin the
+ * lock policy matrix and the window-rollover behaviour without sleeping —
+ * the production code accepts an injected $now for exactly this reason.
+ */
+final class AuthThrottleRepositoryTest extends TestCase
+{
+    private static function at(string $iso): DateTimeImmutable
+    {
+        return new DateTimeImmutable($iso, new DateTimeZone('UTC'));
+    }
+
+    public function testNoLockoutWhenRowMissing(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $this->assertNull(
+            $repo->lockoutFor('1.2.3.4', 'admin@example.com', self::at('2026-05-07T10:00:00Z'))
+        );
+    }
+
+    public function testFirstFourFailuresDoNotLock(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 4; $i++) {
+            $lock = $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+            $this->assertNull($lock, "attempt {$i} must not lock");
+        }
+        $this->assertNull(
+            $repo->lockoutFor('1.2.3.4', 'admin@example.com', $now)
+        );
+    }
+
+    public function testFifthFailureLocksForFiveMinutes(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 4; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        }
+        $lock = $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        $this->assertNotNull($lock);
+        $this->assertSame(
+            '2026-05-07T10:05:00Z',
+            $lock->format('Y-m-d\TH:i:s\Z')
+        );
+    }
+
+    public function testTenthFailureEscalatesToThirtyMinutes(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 9; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        }
+        $lock = $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        $this->assertNotNull($lock);
+        $this->assertSame(
+            '2026-05-07T10:30:00Z',
+            $lock->format('Y-m-d\TH:i:s\Z')
+        );
+    }
+
+    public function testTwentiethFailureEscalatesToOneHour(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 19; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        }
+        $lock = $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        $this->assertNotNull($lock);
+        $this->assertSame(
+            '2026-05-07T11:00:00Z',
+            $lock->format('Y-m-d\TH:i:s\Z')
+        );
+    }
+
+    public function testLockoutExpiresAfterDuration(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $start = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 5; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $start);
+        }
+
+        // Locked at +0..+4:59
+        $this->assertNotNull(
+            $repo->lockoutFor('1.2.3.4', 'admin@example.com', self::at('2026-05-07T10:04:59Z'))
+        );
+        // Unlocked at +5:00 (lock_until is exclusive: until > now ⇒ locked).
+        $this->assertNull(
+            $repo->lockoutFor('1.2.3.4', 'admin@example.com', self::at('2026-05-07T10:05:00Z'))
+        );
+    }
+
+    public function testCounterResetsAfterIdleWindow(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $start = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 4; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $start);
+        }
+        // 16 minutes later → window is stale (15-min idle threshold).
+        $afterIdle = self::at('2026-05-07T10:16:00Z');
+        $lock = $repo->recordFailure('1.2.3.4', 'admin@example.com', $afterIdle);
+        $this->assertNull($lock, 'counter must reset to 1 after idle window');
+    }
+
+    public function testCounterDoesNotResetWithinWindow(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $start = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 4; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $start);
+        }
+        // 14 minutes later → still inside the window.
+        $stillInside = self::at('2026-05-07T10:14:00Z');
+        $lock = $repo->recordFailure('1.2.3.4', 'admin@example.com', $stillInside);
+        $this->assertNotNull($lock, 'fifth attempt within the window must lock');
+    }
+
+    public function testClearRemovesRow(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 5; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        }
+        $this->assertNotNull($repo->lockoutFor('1.2.3.4', 'admin@example.com', $now));
+
+        $repo->clear('1.2.3.4', 'admin@example.com');
+
+        $this->assertNull($repo->lockoutFor('1.2.3.4', 'admin@example.com', $now));
+        $next = $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        $this->assertNull($next, 'after clear the next single failure must not lock');
+    }
+
+    public function testKeysAreCaseInsensitiveOnEmail(): void
+    {
+        // An attacker varying the case of the email shouldn't get extra
+        // attempts (`Admin@…` vs `admin@…`); the bucket is canonicalised.
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 4; $i++) {
+            $repo->recordFailure('1.2.3.4', 'Admin@example.com', $now);
+        }
+        $lock = $repo->recordFailure('1.2.3.4', 'admin@EXAMPLE.com', $now);
+        $this->assertNotNull($lock, 'case-varied email must hit the same bucket');
+    }
+
+    public function testIpsBucketSeparately(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $now  = self::at('2026-05-07T10:00:00Z');
+        for ($i = 1; $i <= 5; $i++) {
+            $repo->recordFailure('1.2.3.4', 'admin@example.com', $now);
+        }
+        // Different IP → not throttled even though the same email is
+        // currently locked from another source.
+        $this->assertNull(
+            $repo->lockoutFor('5.6.7.8', 'admin@example.com', $now),
+            'a second attacker IP must not inherit the first IP\'s lock'
+        );
+    }
+
+    public function testComputeLockoutPolicyMatrix(): void
+    {
+        $now = self::at('2026-05-07T10:00:00Z');
+        $this->assertNull(AuthThrottleRepository::computeLockout(0, $now));
+        $this->assertNull(AuthThrottleRepository::computeLockout(4, $now));
+
+        $r5 = AuthThrottleRepository::computeLockout(5, $now);
+        $this->assertNotNull($r5);
+        $this->assertSame('2026-05-07T10:05:00Z', $r5->format('Y-m-d\TH:i:s\Z'));
+
+        $r9 = AuthThrottleRepository::computeLockout(9, $now);
+        $this->assertNotNull($r9);
+        $this->assertSame('2026-05-07T10:05:00Z', $r9->format('Y-m-d\TH:i:s\Z'));
+
+        $r10 = AuthThrottleRepository::computeLockout(10, $now);
+        $this->assertNotNull($r10);
+        $this->assertSame('2026-05-07T10:30:00Z', $r10->format('Y-m-d\TH:i:s\Z'));
+
+        $r19 = AuthThrottleRepository::computeLockout(19, $now);
+        $this->assertNotNull($r19);
+        $this->assertSame('2026-05-07T10:30:00Z', $r19->format('Y-m-d\TH:i:s\Z'));
+
+        $r20 = AuthThrottleRepository::computeLockout(20, $now);
+        $this->assertNotNull($r20);
+        $this->assertSame('2026-05-07T11:00:00Z', $r20->format('Y-m-d\TH:i:s\Z'));
+
+        $r99 = AuthThrottleRepository::computeLockout(99, $now);
+        $this->assertNotNull($r99);
+        $this->assertSame('2026-05-07T11:00:00Z', $r99->format('Y-m-d\TH:i:s\Z'));
+    }
+
+    public function testPurgeExpiredDropsOnlyStaleRows(): void
+    {
+        $repo = new AuthThrottleRepository($this->makeDb());
+        $repo->recordFailure('1.2.3.4', 'a@x', self::at('2026-05-07T08:00:00Z'));
+        $repo->recordFailure('1.2.3.4', 'b@x', self::at('2026-05-07T11:00:00Z'));
+
+        // Cutoff at 10:00 — 'a@x' (08:00) is older, 'b@x' (11:00) is fresh.
+        $deleted = $repo->purgeExpired(self::at('2026-05-07T10:00:00Z'));
+        $this->assertSame(1, $deleted);
+
+        $this->assertNull(
+            $repo->lockoutFor('1.2.3.4', 'a@x', self::at('2026-05-07T11:30:00Z'))
+        );
+    }
+}

+ 6 - 0
views/auth/local.twig

@@ -15,6 +15,12 @@
             </div>
         {% endif %}
 
+        {% if throttled %}
+            <div class="mt-4 rounded-md border border-amber-200 bg-amber-50 px-3 py-2 text-sm text-amber-800 dark:bg-amber-900 dark:border-amber-800 dark:text-amber-200">
+                Too many failed attempts. Please wait a few minutes before trying again.
+            </div>
+        {% endif %}
+
         <form method="post" action="/auth/local" hx-boost="true" hx-target="body"
               class="mt-4 space-y-3" autocomplete="off">
             <input type="hidden" name="_csrf" value="{{ csrfToken }}">