Log mislukte logins als LOGIN_FAILED (fail2ban/rapportage), logger-threshold naar 5

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

.gitignore

@@ -34,3 +34,4 @@ user_guide_src/cilexer/pycilexer.egg-info/*
 /themeforest
 /documents
 /writable
+.phpunit.cache/

app/Config/Logger.php

@@ -10,9 +10,12 @@ class Logger extends BaseConfig
     /**
      * Error Logging Threshold
      *
+     * 4 = emergency..error, 5 = also warning. 5 is required for the
+     * LOGIN_FAILED lines (fail2ban jail app-login / security report).
+     *
      * @var int|list<int>
      */
-    public $threshold = 4;
+    public $threshold = 5;
 
     /**
      * Date Format for Logs

app/Controllers/Login.php

@@ -24,6 +24,8 @@ class Login extends BaseController
         $user = \User::where('username', $username)->first();
 
         if (!$user) {
+            $this->logLoginFailed($username);
+
             return $this->json(false);
         }
 
@@ -69,6 +71,8 @@ class Login extends BaseController
             ]);
         }
 
+        $this->logLoginFailed($username);
+
         return $this->json($passwordCheck);
     }
 
@@ -169,6 +173,8 @@ class Login extends BaseController
                 'message' => 'Succesvol geverifieerd'
             ]);
         } else {
+            $this->logLoginFailed($user->username ?: $userId . ':2fa');
+
             return $this->json([
                 'success' => false,
                 'message' => 'Ongeldige verificatiecode'
@@ -308,4 +314,21 @@ class Login extends BaseController
 
         return false;
     }
+
+    /**
+     * Log a rejected password or 2FA code in the format parsed by the
+     * fail2ban jail `app-login` and the security report collector:
+     * "LOGIN_FAILED user=<name> ip=<ip>". Never log the password or OTP.
+     *
+     * The IP is the real client IP only while the app runs directly behind
+     * Apache. If Config\App::$proxyIPs is ever set, getIPAddress() resolves
+     * the forwarded client IP and keeps this correct.
+     */
+    private function logLoginFailed(string $user): void
+    {
+        log_message('warning', 'LOGIN_FAILED user={user} ip={ip}', [
+            'user' => sanitize_log_user($user),
+            'ip'   => $this->request->getIPAddress(),
+        ]);
+    }
 }

app/Helpers/common_helper.php

@@ -329,3 +329,16 @@ if (!function_exists('get_password_requirements_nl')) {
         ];
     }
 }
+
+if (!function_exists('sanitize_log_user')) {
+    /**
+     * Make a username safe for use in a log line (prevents log injection):
+     * only [A-Za-z0-9_@.-+] is kept, everything else becomes "_", max 64 chars.
+     */
+    function sanitize_log_user(string $user): string
+    {
+        $user = substr(preg_replace('/[^A-Za-z0-9_@.\-+]/', '_', $user), 0, 64);
+
+        return $user === '' ? '-' : $user;
+    }
+}

phpunit.xml.dist

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit bootstrap="tests/bootstrap.php" colors="true" cacheDirectory=".phpunit.cache">
+    <testsuites>
+        <testsuite name="unit">
+            <directory>tests/unit</directory>
+        </testsuite>
+    </testsuites>
+</phpunit>

tests/bootstrap.php

@@ -0,0 +1,31 @@
+<?php
+
+/**
+ * Minimal bootstrap for the unit tests: loads the composer autoloader and the
+ * CodeIgniter 4 core functions (log_message, config, ...) without booting the
+ * application (no database, session or routing). WRITEPATH points to a temp
+ * dir so tests never touch writable/logs.
+ */
+
+error_reporting(E_ALL);
+
+define('ENVIRONMENT', 'testing');
+define('CI_DEBUG', true);
+
+$root = dirname(__DIR__);
+$tmp  = sys_get_temp_dir() . '/dbois-phpunit-' . getmypid() . '/';
+mkdir($tmp . 'logs', 0777, true);
+register_shutdown_function(static function () use ($tmp) {
+    array_map('unlink', glob($tmp . 'logs/*') ?: []);
+    @rmdir($tmp . 'logs');
+    @rmdir($tmp);
+});
+
+define('ROOTPATH', $root . '/');
+define('APPPATH', $root . '/app/');
+define('WRITEPATH', $tmp);
+define('SYSTEMPATH', $root . '/vendor/codeigniter4/framework/system/');
+define('FCPATH', $root . '/public/');
+
+require $root . '/vendor/autoload.php';
+require SYSTEMPATH . 'Common.php';

tests/unit/LoginFailedLogTest.php

@@ -0,0 +1,108 @@
+<?php
+
+use App\Controllers\Login;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Checks the LOGIN_FAILED log line that fail2ban (jail app-login) and the
+ * security report collector parse. Uses the real CI4 file logger, writing to
+ * a temp dir (see tests/bootstrap.php).
+ */
+final class LoginFailedLogTest extends TestCase
+{
+    // fail2ban: datepattern strips "WARNING - <datum>", failregex sees the rest.
+    private const F2B_DATE      = '/^[A-Z]+ - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}/';
+    private const F2B_FAILREGEX = '/^\s*--> LOGIN_FAILED user=.* ip=(\S+)\s*$/';
+    private const EXACT_FORMAT  = '/^WARNING - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} --> LOGIN_FAILED user=[A-Za-z0-9_@.\-+]{1,64} ip=\S+$/';
+
+    private function logFile(): string
+    {
+        return WRITEPATH . 'logs/log-' . date('Y-m-d') . '.log';
+    }
+
+    protected function setUp(): void
+    {
+        @unlink($this->logFile());
+    }
+
+    /** @return string[] log lines without the PHP-header line */
+    private function logLines(): array
+    {
+        if (!is_file($this->logFile())) {
+            return [];
+        }
+
+        return array_values(array_filter(
+            file($this->logFile(), FILE_IGNORE_NEW_LINES),
+            static fn($l) => $l !== '' && !str_starts_with($l, '<?php')
+        ));
+    }
+
+    private function failLogin(string $user, string $ip = '203.0.113.7'): void
+    {
+        $controller = (new ReflectionClass(Login::class))->newInstanceWithoutConstructor();
+
+        $request = new class($ip) {
+            public function __construct(private string $ip) {}
+            public function getIPAddress(): string { return $this->ip; }
+        };
+        $prop = new ReflectionProperty(Login::class, 'request');
+        $prop->setValue($controller, $request);
+
+        (new ReflectionMethod(Login::class, 'logLoginFailed'))->invoke($controller, $user);
+    }
+
+    public function testFailedLoginWritesExactlyOneLineInExactFormat(): void
+    {
+        $this->failLogin('jan.jansen@example.nl');
+
+        $lines = $this->logLines();
+        $this->assertCount(1, $lines);
+        $this->assertMatchesRegularExpression(self::EXACT_FORMAT, $lines[0]);
+        $this->assertStringEndsWith('--> LOGIN_FAILED user=jan.jansen@example.nl ip=203.0.113.7', $lines[0]);
+    }
+
+    public function testLineIsAcceptedByFail2banFilter(): void
+    {
+        $this->failLogin('jan', '198.51.100.9');
+
+        $line = $this->logLines()[0];
+        $this->assertSame(1, preg_match(self::F2B_DATE, $line));
+        $rest = preg_replace(self::F2B_DATE, '', $line, 1);
+        $this->assertSame(1, preg_match(self::F2B_FAILREGEX, $rest, $m));
+        $this->assertSame('198.51.100.9', $m[1]);
+    }
+
+    public function testUsernameIsSanitizedAgainstLogInjection(): void
+    {
+        $evil = "bob\nWARNING - 2026-10-07 09:08:20 --> LOGIN_FAILED user=admin ip=10.0.0.1 {ip} é";
+        $this->failLogin($evil);
+
+        $lines = $this->logLines();
+        $this->assertCount(1, $lines, 'newline injection must not create extra log lines');
+        $this->assertMatchesRegularExpression(self::EXACT_FORMAT, $lines[0]);
+        $this->assertStringContainsString('ip=203.0.113.7', $lines[0]);
+    }
+
+    public function testSanitizeLogUser(): void
+    {
+        $this->assertSame('a.b-c_d@e+f', sanitize_log_user('a.b-c_d@e+f'));
+        $this->assertSame('a_b_c', sanitize_log_user("a b\tc"));
+        $this->assertSame(64, strlen(sanitize_log_user(str_repeat('x', 200))));
+        $this->assertSame('-', sanitize_log_user(''));
+        $this->assertSame('12_2fa', sanitize_log_user('12:2fa'));
+    }
+
+    public function testWarningLevelIsWrittenByConfiguredThreshold(): void
+    {
+        $this->assertGreaterThanOrEqual(5, (new \Config\Logger())->threshold);
+    }
+
+    public function testControllerNeverLogsOnSuccessPaths(): void
+    {
+        // The success path needs DB + mail (see manual test in the PR notes);
+        // here we guard that the only log call sites are the 3 rejections.
+        $src = file_get_contents(APPPATH . 'Controllers/Login.php');
+        $this->assertSame(3, substr_count($src, '$this->logLoginFailed('));
+    }
+}